Skip to content

docs: design specs for graph utilization gaps (description generation + text2sql) - #77

Closed
mingjerli wants to merge 2 commits into
mainfrom
docs/graph-utilization-designs
Closed

docs: design specs for graph utilization gaps (description generation + text2sql)#77
mingjerli wants to merge 2 commits into
mainfrom
docs/graph-utilization-designs

Conversation

@mingjerli

Copy link
Copy Markdown
Owner

Summary

Adds two design documents specifying fixes for gaps where the LLM-facing features underuse the lineage graph:

  • plans/DESIGN_DESCRIPTION_GENERATION_GRAPH_GAPS.md — 4 specs (D1–D4): source-column cold start (forward-usage prompts behind an opt-in include_sources flag), replacing O(V·E) edge scans with the existing _incoming_index, a new DescriptionSource.FALLBACK state so rule-based placeholders are distinguishable/retryable and excluded from downstream prompts, and unifying target-table resolution with metadata propagation so terminal SELECTs get descriptions.
  • plans/DESIGN_TEXT2SQL_GRAPH_CONTEXT.md — 5 specs (T1–T5): column lineage in default direct mode, transitive (depth-configurable) lineage expansion, join-key surfacing (observed joins from query ASTs + candidate joins from shared lineage sources), final/intermediate/source table role annotation with truncation priority, and a graph-aware keyword-selection fallback.

Each spec includes problem statement with file:line references, design, test cases, risks, and a phase ordering.

Note: plans/ is in .gitignore (line 224) — the existing design docs there are local-only. These two files are force-added (git add -f) per explicit request; if plans/ should stay private, close this and they can move to a tracked location instead.

Test plan

  • Docs only — no code changes; make pre-commit passed (ruff format/lint clean)
  • Review that spec'd APIs match current code references

🤖 Generated with Claude Code

https://claude.ai/code/session_01MCAw3ceZaN7roTDpAuUtYF

@mingjerli

Copy link
Copy Markdown
Owner Author

I reviewed both design specs against the current source and ran focused pipeline probes. I found four assumptions to tighten before implementation:

  1. D1 source prompt dispatch (DESIGN_DESCRIPTION_GENERATION_GRAPH_GAPS.md:81-83): ColumnNode.is_computed() cannot identify source inputs today. _add_query_columns() assigns query.query_id to every parsed node, and is_computed() returns True whenever query_id is present. In a simple raw.users -> staging.users pipeline, raw.users.email has layer="input" and no incoming edges, but is_computed() is still True. The proposed dispatch would therefore use the computed-column prompt for the source columns D1 adds. Classify the prompt from table_graph.tables[column.table_name].is_source (with layer/no-incoming checks as validation), and add a test that asserts which builder is called.

  2. Use one capped table set for every prompt section (DESIGN_TEXT2SQL_GRAPH_CONTEXT.md:22-36): the direct path currently calls build_schema_context(), which applies max_tables internally, then calls get_table_names(), which returns every table. Passing that full list to _build_graph_context would emit relationships, lineage, and join hints for tables omitted from <schema>; omitted tables can also consume caps before included tables receive hints. Resolve the ordered, capped table list once, then use it for schema context, graph context, notes, tables_used, and a regression test with more than max_tables tables.

  3. Define a safe scope for USING joins (DESIGN_TEXT2SQL_GRAPH_CONTEXT.md:160-172): a sqlglot Join identifies the right relation, but its left input can be a composite joined relation. In a JOIN b USING (id) JOIN c USING (id), the second join does not provide a unique physical left_table; choosing a or b would fabricate an observed pair. Emit a hint only when the left side resolves to one physical table. Otherwise skip and log it. Add a chained-USING test to preserve the stated zero-fabrication goal.

  4. Shared ultimate ancestry does not prove an equality join (DESIGN_TEXT2SQL_GRAPH_CONTEXT.md:174-184): trace_column_backward() returns ultimate leaves and discards transformation and grain. For example, mart.user_counts.user_count = COUNT(raw.users.id) and mart.user_ids.user_id = raw.users.id both trace to raw.users.id, so the proposed algorithm suggests user_count = user_id. The candidate: label does not make that join safe for SQL generation. Restrict candidates to identity-preserving/pass-through paths by inspecting full-path edge types and grain, and add this aggregate counterexample. If the graph cannot prove that property, ship observed joins without inferred candidates.

The overall phase split is workable once these correctness cases are specified.

- D1: dispatch source prompts on table_graph is_source, not is_computed()
- T1/shared: resolve one capped table set for schema, graph context, notes
- T3: USING joins only with single-table left input; chained-USING test
- T3: candidates require identity-preserving paths via edge_type allowlist
@mingjerli

Copy link
Copy Markdown
Owner Author

Addressed all four points in the latest commit:

  1. D1 dispatch — now specified on table_graph.tables[column.table_name].is_source, with the is_computed() limitation documented in the spec (returns True for any node with query_id, models.py:570-578). layer == "input" / no-incoming-edges are demoted to test assertions. Added a dispatch test that spies on both builders and pins raw.users.email (with query_id set) to the source builder.

  2. One capped table set — the shared-change section now leads with the current build_schema_context() vs get_table_names() inconsistency and specifies resolve_context_tables() as the single source of truth feeding schema, relationship/lineage/join sections, PII notes, and tables_used; regression test with more than max_tables tables requires every referenced table to appear in <schema>.

  3. USING scope — hints are emitted only when the join's left input resolves to exactly one physical table; composite left inputs (chained USING) are skipped and debug-logged. Chained-USING test added: exactly one hint from a JOIN b USING (id) JOIN c USING (id), no fabricated pairs.

  4. Candidate joins — candidates now require the entire backward path (via trace_column_backward_full) to be identity-preserving: every ColumnEdge.edge_type in {direct, star_passthrough, cross_query}, unknown types fail closed. Your COUNT(raw.users.id) counterexample is in the spec and the test list. Explicit fallback stance: if edge_type granularity can't prove identity preservation in practice, ship observed joins without candidates.

@mingjerli

Copy link
Copy Markdown
Owner Author

Closing — the specs served their purpose during implementation (superseded by PR #78, which implements all 9 features); design docs not needed in the repo.

@mingjerli mingjerli closed this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant