Context
Surfaced during the round-1 review of PR #175 (Closes #164) as advisory findings A4 and A5 (plus the A11(b) note). Both concern reconciling the CLUSTER/MERGE star-passthrough column-resolution toolkit (src/giql/expanders/_genomic.py) with the pass-1 resolver (src/giql/expanders/resolver.py). This is a non-blocking refactor / correctness-hardening follow-up — PR #175 shipped with a documentation-only note per the reviewers' recommended remediations; the actual reconciliation is deferred here.
A4 — Two conflicting models of "what columns does a subquery expose", and the latent bug it hides
For the same input (SELECT * FROM regions) with a custom-mapped regions, the two subsystems disagree:
_genomic._resolve_query traces a SELECT * / alias.* FROM source through to the underlying table's physical genomic columns, honoring custom mappings. This is the more correct model.resolver.py's reference-slot machinery — _resolve_disjoin_reference (~731-799) and validate_projection_contracts (~1175-1344) — assumes canonical columns for a subquery/CTE reference operand, and even passes a SELECT * through as canonical.
CLUSTER/MERGE are deliberately carved out of the reference-slot framework, so the divergence is defensible for those operators — but the resolver's assume-canonical model is a latent bug for the operators that do use the reference path: DISJOIN and NEAREST currently assume canonical columns and would emit phantom columns (chrom/start/end) for a SELECT *-over-a-custom-mapped-table reference, rather than the table's actual physical columns.
Follow-up: Teach the resolver's reference path to trace star-passthrough sources the same way _genomic._resolve_query does. This reconciles the two models onto the more-correct (trace-through) one and fixes the latent DISJOIN/NEAREST SELECT *-over-custom-mapped-table bug.
A5 — The toolkit hand-rolls source/CTE/mapping logic the resolver already owns
_genomic.py re-implements machinery the pass-1 resolver already provides, inviting drift:
_find_cte re-introduces a hand-rolled .parent / with_ ancestor walk that resolver.py deliberately replaced with scope.cte_sources._mapping_for duplicates resolver._physical_cols (~709-728).- The star / canonical-subset check in
_resolve_query mirrors resolver._static_output_columns (~1280-1308), with inverted polarity.
Follow-up: Extract shared leaf helpers — a common physical_columns(table) and static_output_columns() — that both subsystems consume, so the two do not drift apart.
Related (A11(b)): _genomic._CANONICAL_COLUMNS is a third representation of the canonical-default columns, alongside resolver._DEFAULT_COLS and resolver._physical_cols(None). It is a candidate to unify onto the shared helper if the extraction above lands.
References
https://claude.ai/code/session_01KAYsMtN7vYuECZHeeFFzCM
Context
Surfaced during the round-1 review of PR #175 (Closes #164) as advisory findings A4 and A5 (plus the A11(b) note). Both concern reconciling the CLUSTER/MERGE star-passthrough column-resolution toolkit (
src/giql/expanders/_genomic.py) with the pass-1 resolver (src/giql/expanders/resolver.py). This is a non-blocking refactor / correctness-hardening follow-up — PR #175 shipped with a documentation-only note per the reviewers' recommended remediations; the actual reconciliation is deferred here.A4 — Two conflicting models of "what columns does a subquery expose", and the latent bug it hides
For the same input
(SELECT * FROM regions)with a custom-mappedregions, the two subsystems disagree:_genomic._resolve_querytraces aSELECT */alias.*FROM source through to the underlying table's physical genomic columns, honoring custom mappings. This is the more correct model.resolver.py's reference-slot machinery —_resolve_disjoin_reference(~731-799) andvalidate_projection_contracts(~1175-1344) — assumes canonical columns for a subquery/CTE reference operand, and even passes aSELECT *through as canonical.CLUSTER/MERGE are deliberately carved out of the reference-slot framework, so the divergence is defensible for those operators — but the resolver's assume-canonical model is a latent bug for the operators that do use the reference path: DISJOIN and NEAREST currently assume canonical columns and would emit phantom columns (
chrom/start/end) for aSELECT *-over-a-custom-mapped-table reference, rather than the table's actual physical columns.Follow-up: Teach the resolver's reference path to trace star-passthrough sources the same way
_genomic._resolve_querydoes. This reconciles the two models onto the more-correct (trace-through) one and fixes the latent DISJOIN/NEARESTSELECT *-over-custom-mapped-table bug.A5 — The toolkit hand-rolls source/CTE/mapping logic the resolver already owns
_genomic.pyre-implements machinery the pass-1 resolver already provides, inviting drift:_find_ctere-introduces a hand-rolled.parent/with_ancestor walk thatresolver.pydeliberately replaced withscope.cte_sources._mapping_forduplicatesresolver._physical_cols(~709-728)._resolve_querymirrorsresolver._static_output_columns(~1280-1308), with inverted polarity.Follow-up: Extract shared leaf helpers — a common
physical_columns(table)andstatic_output_columns()— that both subsystems consume, so the two do not drift apart.Related (A11(b)):
_genomic._CANONICAL_COLUMNSis a third representation of the canonical-default columns, alongsideresolver._DEFAULT_COLSandresolver._physical_cols(None). It is a candidate to unify onto the shared helper if the extraction above lands.References
.sdlc/reviews/issue-#164/review-1.mdhttps://claude.ai/code/session_01KAYsMtN7vYuECZHeeFFzCM