feat(ibis_yaml): serialize TeeNode and its WriteThrough writer

[mesejo]

Summary

Adds ibis-yaml serialization for TeeNode (the transparent pass-through behind .tee(), ADR-0014), which previously had no translate_to_yaml/from_yaml rule.

• Round-trips parent, drain, and the writer. All four WriteThrough types covered:
  • ParquetWriteThrough → path, mode (filesystem, no connection)
  • Backend / ThreadedBackendWriteThrough → connection as profile hash, table_name, mode
  • WritePrimaryWriteThrough → recurses into inner
  • Identity-neutral transport tuning (kwargs/maxsize) is omitted so expr.yaml stays byte-stable across rebuilds.
• Profile plumbing for backend-bound writers. A backend writer holds a live connection, so it must join the profile machinery: WriteThrough gains iter_cons()/replace_cons(), TeeNode joins find_all_sources (via _node_cons), and replace_sources rebuilds the writer during profile canonicalization. The writer's connection is now discovered, dehydrated to profiles.yaml, and rehydrated on load.
• SQL transparency. to_sql now strips TeeNode via the existing _remove_tee_nodes before compiling. TeeNode is a bare ops.Relation with no SQL compiler visitor and, unlike RemoteTable, was not elided — so build_expr crashed while generating SQL metadata. This mirrors the .sql() view path.

Testing

New python/xorq/ibis_yaml/tests/test_tee.py (6 tests): parquet / backend / threaded / write-primary writers, mode+drain preservation, byte-stable round-trip, and full build_expr/load_expr. All pass.

Regression: test_write_through, test_letsql_ops, test_transform_scope, test_compiler, test_basic, test_sql all green (only pre-existing postgres tests fail locally — no DB / docker daemon down).

Note: duckdb tee().execute() deadlocks (ADR-0014 Phase-1 limit), so execute-path tests use a datafusion parent. Serialization itself is backend-agnostic.

🤖 Generated with Claude Code

[roborev-xorq-labs]

roborev: Fail

Review findings

Review Findings

Severity: Medium
Location: python/xorq/writes/write_through.py:207-213 / python/xorq/ibis_yaml/translate.py:574-581
Problem: ThreadedBackendWriteThrough has a maxsize field that is identity-neutral (marked hash=False, eq=False) and intentionally omitted from serialization. WritePrimaryWriteThrough also has an identity-neutral maxsize. The docstring acknowledges this. However, the deserialization path reconstructs both classes using only the identity-bearing fields, which means any runtime-configured maxsize on an existing TeeNode is silently dropped across a build_expr/load_expr cycle. For bounded-queue deployments this changes backpressure behavior without any warning. At minimum a comment in load_writer_from_yaml should note the deliberate omission so callers know what they're getting.
Fix: Add a comment in load_writer_from_yaml (parallel to the comment already in translate_writer) noting that maxsize defaults to 0 on load, and consider whether the load-time default should be configurable.

---

Severity: Low
Location: python/xorq/ibis_yaml/sql.py:29
Problem: _remove_tee_nodes is imported by private name (leading underscore) from xorq.expr.api inside the function body. A rename of the private helper breaks to_sql at call time rather than import time, making the failure harder to catch in CI if the function is not exercised in tests that also call to_sql.
Fix: Either expose _remove_tee_nodes as a public API (rename to remove_tee_nodes) or import it at module level so import-time failures surface early.

---

Severity: Low
Location: python/xorq/ibis_yaml/tests/test_tee.py:80-91
Problem: test_backend_tee_roundtrip does not assert that op.writer.mode survives the roundtrip, unlike test_parquet_tee_roundtrip (which checks op.writer.mode is WriteMode.CREATE) and test_parquet_tee_preserves_mode_and_drain (which checks op.writer.mode is WriteMode.APPEND). The mode field is serialized for BackendWriteThrough but the test provides no coverage that it is correctly restored.
Fix: Add assert op.writer.mode is WriteMode.CREATE (or whatever the default is) to test_backend_tee_roundtrip, and a symmetric test for a non-default mode value.

Summary

Adds YAML serialization/deserialization for TeeNode and its WriteThrough writer variants, together with iter_cons/replace_cons hooks on the writer hierarchy for source discovery and profile canonicalization.

---

Review type: default | Agent: claude-code | Job: 786

[codecov]

Codecov Report

❌ Patch coverage is 97.14286% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
python/xorq/ibis_yaml/translate.py	95.12%	2 Missing ⚠️

@@            Coverage Diff             @@
##             main    #2152      +/-   ##
==========================================
+ Coverage   82.27%   82.42%   +0.15%     
==========================================
  Files         157      157              
  Lines       19770    19834      +64     
==========================================
+ Hits        16265    16349      +84     
+ Misses       3505     3485      -20     

Flag	Coverage Δ
catalog-annex	46.33% <37.14%> (-0.05%)	⬇️
catalog-git	51.66% <37.14%> (-0.07%)	⬇️
core	67.07% <97.14%> (+0.20%)	⬆️
library	0.44% <0.00%> (-0.01%)	⬇️
slow	53.66% <37.14%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
python/xorq/common/utils/graph_utils.py	97.45% <100.00%> (+0.08%)	⬆️
python/xorq/ibis_yaml/sql.py	100.00% <100.00%> (ø)
python/xorq/writes/write_through.py	98.22% <100.00%> (+0.09%)	⬆️
python/xorq/ibis_yaml/translate.py	89.64% <95.12%> (+0.38%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[roborev-xorq-labs]

roborev: Fail

Review findings

Review Findings

Severity: Low
Location: python/xorq/ibis_yaml/translate.py:568–582
Problem: translate_writer for WritePrimaryWriteThrough omits maxsize, and the ThreadedBackendWriteThrough/BackendWriteThrough branch also omits kwargs. The docstring acknowledges this as intentional ("identity-neutral"), but the YAML roundtrip silently drops transport-tuning configuration set by callers. A user who configured a bounded queue with maxsize=4 would get the default maxsize=0 after a roundtrip with no warning.
Fix: Either document this in the public API (e.g., raise if non-default values are present), or add a comment in the tests noting the known gap so it doesn't look like an oversight.

---

Severity: Low
Location: python/xorq/ibis_yaml/tests/test_tee.py (missing tests)
Problem: The error paths in translate_writer (NotImplementedError for unknown writer type) and load_writer_from_yaml (ValueError for unknown kind) are not exercised. If a new WriteThrough subclass is added without registering it in translate_writer, the failure mode is a runtime error rather than a CI failure.
Fix: Add a test with a stub WriteThrough subclass to verify both error branches are hit and raise the expected exceptions.

---

Severity: Low
Location: python/xorq/ibis_yaml/sql.py:36
Problem: The inline comment # xorq-style: disable=protected-access is not a recognized linting-suppression format for ruff (which uses # noqa: SLF001). It reads as a comment rather than an effective directive, so the linter will still flag _find_backend.
Fix: Replace with # noqa: SLF001 if suppression is actually needed, or remove the comment since _find_backend was already called without suppression in the original code.

Summary

Adds YAML serialization round-trip support for TeeNode and all four WriteThrough variants (Parquet, Backend, ThreadedBackend, WritePrimary), wiring them into graph_utils source-discovery and replace_sources traversal, with comprehensive round-trip tests covering each writer type.

---

Review type: default | Agent: claude-code | Job: 823

[roborev-xorq-labs]

roborev: Fail

Review findings

Review Findings

Severity: Low
Location: python/xorq/ibis_yaml/sql.py:44-45
Problem: The lint-suppression comment # xorq-style: disable=protected-access is placed on the closing-parenthesis line rather than on the line containing the private method call uncached._find_backend(. As written it suppresses nothing; the same misplacement appears in translate.py:566.
Fix: Move the comment to the line that calls _find_backend (or writer.con._profile), or use the project's actual linter directive format (# noqa: SLF001).

---

Severity: Low
Location: python/xorq/ibis_yaml/tests/test_tee.py — no test for WritePrimaryWriteThrough(inner=BackendWriteThrough(...))
Problem: test_write_primary_tee_roundtrip only exercises the parquet inner path. The WritePrimaryWriteThrough.iter_cons / replace_cons delegation to a backend-bound inner writer (the path that actually needs con-discovery) has no coverage.
Fix: Add a test that wraps BackendWriteThrough in WritePrimaryWriteThrough, does a YAML roundtrip, and asserts op.writer.inner.con is target.

---

Severity: Low
Location: python/xorq/ibis_yaml/tests/test_tee.py — test_backend_tee_roundtrip and test_threaded_backend_tee_roundtrip
Problem: Neither test checks that mode survives the YAML roundtrip for backend-bound writers. The parquet path has an explicit test_parquet_tee_preserves_mode_and_drain but the BackendWriteThrough/ThreadedBackendWriteThrough paths do not, leaving the mode serialization in translate_writer untested end-to-end.
Fix: Assert op.writer.mode is WriteMode.CREATE (or a non-default value) in the existing backend roundtrip tests.

Summary

Adds YAML serialization/deserialization of TeeNode and its WriteThrough writers, together with backend-con discovery and substitution hooks; findings are three minor issues around a misplaced lint comment and two testing gaps in the writer-mode and WritePrimaryWriteThrough-wrapping-backend paths.

---

Review type: default | Agent: claude-code | Job: 826

[roborev-xorq-labs]

roborev: Fail

Review findings

All relevant non-TeeNode types inherit from DatabaseTable which has a source attribute. The fallback in _node_cons is safe.

Review Findings

Severity: Low
Location: python/xorq/ibis_yaml/translate.py, translate_writer function (~line 554, ParquetWriteThrough branch)
Problem: warnings.warn(..., stacklevel=2) is correct when translate_writer is called directly from _tee_node_to_yaml, but when called recursively through the WritePrimaryWriteThrough branch (translate_writer(writer.inner, context)), the stacklevel=2 points at the internal recursive call site inside translate_writer rather than the original external caller. The warning will appear to originate from translate_writer's own code rather than the user-facing call chain.
Fix: Use stacklevel=3 or factor the parquet warning into a separate helper, or document the limitation.

---

Severity: Low
Location: python/xorq/ibis_yaml/sql.py line ~35; python/xorq/ibis_yaml/translate.py line ~573
Problem: # xorq-style: disable=protected-access is not a recognized linter directive for pylint, ruff, or any standard tool. It does not suppress any warning and reads as if it should. The existing codebase pattern (seen in catalog.py) uses # noqa: SLF001 (ruff) alongside the custom comment; the new additions omit the # noqa: SLF001 part.
Fix: Add the standard suppression: # noqa: SLF001 # xorq-style: disable=protected-access to match the established pattern in catalog.py.

---

Summary

Adds YAML serialization and deserialization support for TeeNode and its WriteThrough writers, with corresponding source-discovery wiring in graph_utils and a new test suite covering roundtrip, write side-effects, and error paths.

---

Review type: default | Agent: claude-code | Job: 828