Rifqi Hafizuddin Claude Opus 4.8 commited on
Commit
446c575
Β·
1 Parent(s): 781b93c

/fix surface catalog foreign keys to the planner (FK-backed joins)

Browse files

The slow-path planner's CatalogSummary omitted foreign_keys, but the IR validator
only allows FK-backed joins. So join-requiring questions ("which product made the
most revenue") made the LLM guess joins that were rejected 3x β†’ slow path failed.
The prompt already promised FKs; the rendered catalog just never included them.

- inputs.py: CatalogSummary carries foreign_keys + renders FK lines with the exact
join ids (target_table_id/left_column_id/right_column_id) to copy verbatim.
- planner.md: join ONLY on a listed FK; copy ids; don't invent a join (single
table or say the data isn't linked).
- render.py (single-query planner): FK lines now include the join ids too (were
name-only) β€” keeps both planners consistent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/agents/planner/inputs.py CHANGED
@@ -31,11 +31,24 @@ class ColumnSummary(BaseModel):
31
  top_values: list[Any] | None = None
32
 
33
 
 
 
 
 
 
 
 
 
 
 
 
 
34
  class TableSummary(BaseModel):
35
  table_id: str
36
  name: str
37
  row_count: int | None = None
38
  columns: list[ColumnSummary] = Field(default_factory=list)
 
39
 
40
 
41
  class StructuredSourceSummary(BaseModel):
@@ -89,6 +102,16 @@ class CatalogSummary(BaseModel):
89
  )
90
  for col in table.columns
91
  ],
 
 
 
 
 
 
 
 
 
 
92
  )
93
  for table in source.tables
94
  ]
@@ -111,6 +134,12 @@ class CatalogSummary(BaseModel):
111
  lines: list[str] = []
112
  for source in self.structured_sources:
113
  lines.append(f"Source: {source.name} ({source.source_type}) β€” id={source.source_id}")
 
 
 
 
 
 
114
  for table in source.tables:
115
  rc = f" ({table.row_count:,} rows)" if table.row_count is not None else ""
116
  lines.append(f" Table: {table.name}{rc} β€” id={table.table_id}")
@@ -121,6 +150,16 @@ class CatalogSummary(BaseModel):
121
  f" - {col.name} [{col.data_type}]: "
122
  f"samples={samples}{top} β€” id={col.column_id}"
123
  )
 
 
 
 
 
 
 
 
 
 
124
  lines.append("")
125
 
126
  if self.unstructured_sources:
 
31
  top_values: list[Any] | None = None
32
 
33
 
34
+ class ForeignKeySummary(BaseModel):
35
+ """A declared FK edge β€” the only joins the IR validator accepts.
36
+
37
+ Maps directly onto a `retrieve_data` IR join: `column_id` β†’ `left_column_id`,
38
+ `target_table_id` β†’ `target_table_id`, `target_column_id` β†’ `right_column_id`.
39
+ """
40
+
41
+ column_id: str
42
+ target_table_id: str
43
+ target_column_id: str
44
+
45
+
46
  class TableSummary(BaseModel):
47
  table_id: str
48
  name: str
49
  row_count: int | None = None
50
  columns: list[ColumnSummary] = Field(default_factory=list)
51
+ foreign_keys: list[ForeignKeySummary] = Field(default_factory=list)
52
 
53
 
54
  class StructuredSourceSummary(BaseModel):
 
102
  )
103
  for col in table.columns
104
  ],
105
+ # The declared FKs β€” the only joins the validator accepts. FKs
106
+ # carry no PII (ids only), so they're always surfaced.
107
+ foreign_keys=[
108
+ ForeignKeySummary(
109
+ column_id=fk.column_id,
110
+ target_table_id=fk.target_table_id,
111
+ target_column_id=fk.target_column_id,
112
+ )
113
+ for fk in table.foreign_keys
114
+ ],
115
  )
116
  for table in source.tables
117
  ]
 
134
  lines: list[str] = []
135
  for source in self.structured_sources:
136
  lines.append(f"Source: {source.name} ({source.source_type}) β€” id={source.source_id}")
137
+ # Name lookups (within a source) so FK edges render with readable
138
+ # table/column names alongside the ids the IR join must copy verbatim.
139
+ table_name_by_id = {t.table_id: t.name for t in source.tables}
140
+ col_name_by_id = {
141
+ c.column_id: c.name for t in source.tables for c in t.columns
142
+ }
143
  for table in source.tables:
144
  rc = f" ({table.row_count:,} rows)" if table.row_count is not None else ""
145
  lines.append(f" Table: {table.name}{rc} β€” id={table.table_id}")
 
150
  f" - {col.name} [{col.data_type}]: "
151
  f"samples={samples}{top} β€” id={col.column_id}"
152
  )
153
+ for fk in table.foreign_keys:
154
+ tgt_table = table_name_by_id.get(fk.target_table_id, fk.target_table_id)
155
+ tgt_col = col_name_by_id.get(fk.target_column_id, fk.target_column_id)
156
+ src_col = col_name_by_id.get(fk.column_id, fk.column_id)
157
+ lines.append(
158
+ f" FK: {src_col} β†’ {tgt_table}.{tgt_col} "
159
+ f"(join: target_table_id={fk.target_table_id}, "
160
+ f"left_column_id={fk.column_id}, "
161
+ f"right_column_id={fk.target_column_id})"
162
+ )
163
  lines.append("")
164
 
165
  if self.unstructured_sources:
src/catalog/render.py CHANGED
@@ -65,5 +65,11 @@ def render_source(source: Source) -> str:
65
  tgt_col_name = col_names_by_id.get(fk.target_table_id, {}).get(
66
  fk.target_column_id, fk.target_column_id
67
  )
68
- lines.append(f" - {src_col_name} -> {tgt_table_name}.{tgt_col_name}")
 
 
 
 
 
 
69
  return "\n".join(lines)
 
65
  tgt_col_name = col_names_by_id.get(fk.target_table_id, {}).get(
66
  fk.target_column_id, fk.target_column_id
67
  )
68
+ # Include the join ids inline β€” the planner must copy these verbatim
69
+ # into the IR join, and the IRValidator does a literal id lookup.
70
+ lines.append(
71
+ f" - {src_col_name} -> {tgt_table_name}.{tgt_col_name} "
72
+ f"(join: target_table_id={fk.target_table_id}, "
73
+ f"left_column_id={fk.column_id}, right_column_id={fk.target_column_id})"
74
+ )
75
  return "\n".join(lines)
src/config/prompts/planner.md CHANGED
@@ -41,15 +41,20 @@ only a `TaskList` object that conforms to the provided schema.
41
  (referencing the upstream result's column aliases).
42
  - **Measure by a dimension in another table (joins).** When the number you are
43
  aggregating and the grouping dimension live in DIFFERENT tables of the same
44
- database source, add a `joins` entry to the `retrieve_data` IR along a foreign
45
- key declared in the catalog β€” do NOT pick a table that lacks the measure, and do
46
- NOT try to "combine" unrelated tables. Example β€” "revenue by category": the
47
- measure `order_items.line_total` joined to `products` on
48
- `order_items.product_id = products.id`, grouped by `products.category`. Prefer an
49
- existing measure column over recomputing; use a single table (no join) when the
50
- measure and dimension already live together (e.g. "revenue by region" from
51
- `orders.region` + `orders.total_amount`). Joins are database-only β€” not available
52
- for tabular/file sources.
 
 
 
 
 
53
  - **Mixing structured + unstructured.** If qualitative context helps, add a
54
  `retrieve_knowledge` task against an unstructured source listed in the catalog.
55
  - **CRISP-DM stages.** Tag each task with the stage it serves:
 
41
  (referencing the upstream result's column aliases).
42
  - **Measure by a dimension in another table (joins).** When the number you are
43
  aggregating and the grouping dimension live in DIFFERENT tables of the same
44
+ database source, add a `joins` entry to the `retrieve_data` IR. **Join ONLY on a
45
+ foreign key listed in the catalog.** Each joinable relationship appears as an
46
+ `FK:` line under its table, e.g.
47
+ `FK: product_id β†’ products.id (join: target_table_id=t_products, left_column_id=c_oi_product_id, right_column_id=c_products_id)`
48
+ β€” copy those three ids verbatim into the join (`target_table_id`,
49
+ `left_column_id`, `right_column_id`). Example β€” "revenue by category": the measure
50
+ `order_items.line_total` joined to `products` on `order_items.product_id =
51
+ products.id`, grouped by `products.category`. **If no `FK:` line links the tables
52
+ you need, do NOT invent a join** β€” the validator rejects any join that isn't a
53
+ declared FK. Instead use a single table when the measure and dimension already
54
+ live together (e.g. "revenue by region" from `orders.region` +
55
+ `orders.total_amount`); if they genuinely aren't linked, say the data isn't
56
+ connected rather than guessing. Prefer an existing measure column over
57
+ recomputing. Joins are database-only β€” not available for tabular/file sources.
58
  - **Mixing structured + unstructured.** If qualitative context helps, add a
59
  `retrieve_knowledge` task against an unstructured source listed in the catalog.
60
  - **CRISP-DM stages.** Tag each task with the stage it serves: