sofhiaazzhr Claude Opus 4.8 commited on
Commit
aacd4d1
·
1 Parent(s): b1b1017

[NOTICKET] fix(planner): repair near-miss catalog ids in QueryIR before validation

Browse files

The planner LLM occasionally drops, inserts, or mutates a single character
when copying an opaque catalog id (e.g. c_b7489c7a4b5 instead of
c_b7489c7a4b5f). The IRValidator then rejects the IR on an exact-match
lookup, the planner retries with the same typo, and the analysis aborts
after N attempts ("planner failed validation after 3 attempts").

Add IRRepairer (src/query/ir/repair.py): before validation, rewrite an
unresolvable id to the unique catalog id within edit-distance 1, covering
dropped/inserted/substituted chars. When zero or 2+ candidates match, the
id is left untouched so the validator still fails loudly -- it never
guesses. Because catalog ids are fixed-length hashes, ambiguity is
astronomically unlikely, and the unique-or-refuse rule is the guarantee.

Wired before validate at all three IR entry points: the planner validator
(writes the repaired IR back into the tool call so the executor runs the
fixed IR, not just the validator), QueryService, and the retrieve_data
tool. Each repair is logged (ir_repair from->to) for observability.

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

src/agents/planner/validator.py CHANGED
@@ -13,14 +13,19 @@ from __future__ import annotations
13
 
14
  from pydantic import ValidationError
15
 
 
 
16
  from ...catalog.models import Catalog
17
  from ...query.ir.models import QueryIR
 
18
  from ...query.ir.validator import IRValidationError, IRValidator
19
  from .contracts import ToolRegistry
20
  from .errors import PlannerValidationError
21
  from .inputs import Constraints
22
  from .schemas import PLACEHOLDER_RE, TaskList
23
 
 
 
24
  # Heuristic: a checkable success_criteria mentions a measurable signal.
25
  _CHECKABLE_TOKENS = ("rate", "count", "match", "produced", "above", "below", "equal")
26
 
@@ -29,8 +34,13 @@ _WHITE, _GREY, _BLACK = 0, 1, 2
29
 
30
 
31
  class PlannerValidator:
32
- def __init__(self, ir_validator: IRValidator | None = None) -> None:
 
 
 
 
33
  self._ir_validator = ir_validator or IRValidator()
 
34
 
35
  def validate(
36
  self,
@@ -123,6 +133,23 @@ class PlannerValidator:
123
  raise PlannerValidationError(
124
  f"task {task_id}: retrieve_data.args.ir is not a valid QueryIR: {e}"
125
  ) from e
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
126
  try:
127
  self._ir_validator.validate(ir, catalog)
128
  except IRValidationError as e:
 
13
 
14
  from pydantic import ValidationError
15
 
16
+ from src.middlewares.logging import get_logger
17
+
18
  from ...catalog.models import Catalog
19
  from ...query.ir.models import QueryIR
20
+ from ...query.ir.repair import IRRepairer
21
  from ...query.ir.validator import IRValidationError, IRValidator
22
  from .contracts import ToolRegistry
23
  from .errors import PlannerValidationError
24
  from .inputs import Constraints
25
  from .schemas import PLACEHOLDER_RE, TaskList
26
 
27
+ logger = get_logger("ir_repair")
28
+
29
  # Heuristic: a checkable success_criteria mentions a measurable signal.
30
  _CHECKABLE_TOKENS = ("rate", "count", "match", "produced", "above", "below", "equal")
31
 
 
34
 
35
 
36
  class PlannerValidator:
37
+ def __init__(
38
+ self,
39
+ ir_validator: IRValidator | None = None,
40
+ ir_repairer: IRRepairer | None = None,
41
+ ) -> None:
42
  self._ir_validator = ir_validator or IRValidator()
43
+ self._ir_repairer = ir_repairer or IRRepairer()
44
 
45
  def validate(
46
  self,
 
133
  raise PlannerValidationError(
134
  f"task {task_id}: retrieve_data.args.ir is not a valid QueryIR: {e}"
135
  ) from e
136
+
137
+ # Canonicalize near-miss ids (LLM dropped/mutated a char in an opaque
138
+ # catalog id) before validating. On a successful repair, write the fixed
139
+ # IR back into the tool call so the downstream executor runs the
140
+ # corrected IR — not just the validator.
141
+ ir, repairs = self._ir_repairer.repair(ir, catalog)
142
+ if repairs:
143
+ args["ir"] = ir.model_dump()
144
+ for r in repairs:
145
+ logger.info(
146
+ "repaired ir id",
147
+ task_id=task_id,
148
+ where=r.where,
149
+ from_id=r.from_id,
150
+ to_id=r.to_id,
151
+ )
152
+
153
  try:
154
  self._ir_validator.validate(ir, catalog)
155
  except IRValidationError as e:
src/query/ir/repair.py ADDED
@@ -0,0 +1,215 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ """IRRepairer — canonicalize near-miss ids in a QueryIR before validation.
2
+
3
+ The planner LLM must copy opaque catalog ids (``c_<12hex>`` for columns, and the
4
+ analogous ``t_``/``s_`` ids for tables/sources) verbatim into the IR. It
5
+ occasionally drops, inserts, or mutates a single character — e.g. it emits
6
+ ``c_b7489c7a4b5`` where the catalog holds ``c_b7489c7a4b5f``. The IRValidator
7
+ then rejects the IR on an exact-match lookup, the planner retries, produces the
8
+ same typo, and the whole analysis fails after N attempts.
9
+
10
+ This stage sits *before* the validator and rewrites each unresolvable id to the
11
+ catalog id within edit-distance 1 — but only when that match is **unique**. When
12
+ zero or 2+ candidates match, it leaves the id untouched so the validator still
13
+ fails loudly. It never guesses: the worst case is exactly the pre-repair
14
+ behaviour (a hard validation error), never a silently-wrong column.
15
+
16
+ Because every catalog id is a fixed-length hash (``c_`` + 12 hex), the candidate
17
+ space is uniform and collisions within a single table are astronomically
18
+ unlikely — see the ownership discussion in the ticket. The unique-or-refuse rule
19
+ is the real safety guarantee regardless.
20
+
21
+ The repairer is best-effort and pure: it returns a repaired *copy* plus the list
22
+ of edits it made (for observability). It resolves source_id first, then
23
+ table_id, then assembles the column set from the base table plus any joined
24
+ tables (column_ids are globally unique, so the union is safe), then repairs
25
+ every column reference. It never raises — anything it cannot resolve is left for
26
+ the validator to report.
27
+ """
28
+
29
+ from __future__ import annotations
30
+
31
+ from dataclasses import dataclass
32
+
33
+ from ...catalog.models import Catalog, Source, Table
34
+ from .models import QueryIR
35
+
36
+
37
+ @dataclass(frozen=True)
38
+ class Repair:
39
+ """One id rewrite the repairer applied."""
40
+
41
+ where: str # e.g. "select[2].column_id"
42
+ from_id: str
43
+ to_id: str
44
+
45
+
46
+ class IRRepairer:
47
+ """Rewrite near-miss ids in a QueryIR to their unique catalog match."""
48
+
49
+ def repair(self, ir: QueryIR, catalog: Catalog) -> tuple[QueryIR, list[Repair]]:
50
+ """Return a repaired copy of `ir` and the list of edits applied.
51
+
52
+ Safe by construction: an id is only rewritten when exactly one catalog
53
+ id lies within edit-distance 1. Ambiguous or unresolvable ids are left
54
+ as-is for the validator to reject.
55
+ """
56
+ repairs: list[Repair] = []
57
+ ir = ir.model_copy(deep=True)
58
+
59
+ # --- source_id -------------------------------------------------------
60
+ source = _find_source(catalog, ir.source_id)
61
+ if source is None:
62
+ fixed = _unique_near(ir.source_id, [s.source_id for s in catalog.sources])
63
+ if fixed is not None:
64
+ repairs.append(Repair("source_id", ir.source_id, fixed))
65
+ ir.source_id = fixed
66
+ source = _find_source(catalog, fixed)
67
+ if source is None:
68
+ # Unknown source and no unique fix — nothing else is resolvable.
69
+ return ir, repairs
70
+
71
+ known_table_ids = [t.table_id for t in source.tables]
72
+
73
+ # --- table_id --------------------------------------------------------
74
+ if ir.table_id not in known_table_ids:
75
+ fixed = _unique_near(ir.table_id, known_table_ids)
76
+ if fixed is not None:
77
+ repairs.append(Repair("table_id", ir.table_id, fixed))
78
+ ir.table_id = fixed
79
+
80
+ # --- join target_table_id (before assembling columns) ----------------
81
+ for k, join in enumerate(ir.joins):
82
+ if join.target_table_id not in known_table_ids:
83
+ fixed = _unique_near(join.target_table_id, known_table_ids)
84
+ if fixed is not None:
85
+ repairs.append(
86
+ Repair(f"joins[{k}].target_table_id", join.target_table_id, fixed)
87
+ )
88
+ join.target_table_id = fixed
89
+
90
+ # --- assemble the column set (base + joined tables) ------------------
91
+ # column_ids are globally unique (hash of table/col name), so a union of
92
+ # every table in play is a safe candidate set for column repairs.
93
+ tables_in_play: list[Table] = []
94
+ base = _find_table(source, ir.table_id)
95
+ if base is not None:
96
+ tables_in_play.append(base)
97
+ for join in ir.joins:
98
+ tgt = _find_table(source, join.target_table_id)
99
+ if tgt is not None:
100
+ tables_in_play.append(tgt)
101
+ known_col_ids = [c.column_id for t in tables_in_play for c in t.columns]
102
+ if not known_col_ids:
103
+ return ir, repairs
104
+
105
+ # --- column references ----------------------------------------------
106
+ select_aliases = {s.alias for s in ir.select if s.alias}
107
+
108
+ for i, item in enumerate(ir.select):
109
+ # AggSelect may carry column_id=None (COUNT(*)) — nothing to repair.
110
+ if item.column_id is not None:
111
+ fixed = _maybe_fix(item.column_id, known_col_ids)
112
+ if fixed is not None:
113
+ repairs.append(Repair(f"select[{i}].column_id", item.column_id, fixed))
114
+ item.column_id = fixed
115
+
116
+ for i, f in enumerate(ir.filters):
117
+ fixed = _maybe_fix(f.column_id, known_col_ids)
118
+ if fixed is not None:
119
+ repairs.append(Repair(f"filters[{i}].column_id", f.column_id, fixed))
120
+ f.column_id = fixed
121
+
122
+ for i, col_id in enumerate(ir.group_by):
123
+ fixed = _maybe_fix(col_id, known_col_ids)
124
+ if fixed is not None:
125
+ repairs.append(Repair(f"group_by[{i}]", col_id, fixed))
126
+ ir.group_by[i] = fixed
127
+
128
+ for i, ob in enumerate(ir.order_by):
129
+ # order_by may legitimately reference a select alias, not a column.
130
+ # Never rewrite an alias reference.
131
+ if ob.column_id in select_aliases:
132
+ continue
133
+ fixed = _maybe_fix(ob.column_id, known_col_ids)
134
+ if fixed is not None:
135
+ repairs.append(Repair(f"order_by[{i}].column_id", ob.column_id, fixed))
136
+ ob.column_id = fixed
137
+
138
+ for k, join in enumerate(ir.joins):
139
+ fixed = _maybe_fix(join.left_column_id, known_col_ids)
140
+ if fixed is not None:
141
+ repairs.append(
142
+ Repair(f"joins[{k}].left_column_id", join.left_column_id, fixed)
143
+ )
144
+ join.left_column_id = fixed
145
+ fixed = _maybe_fix(join.right_column_id, known_col_ids)
146
+ if fixed is not None:
147
+ repairs.append(
148
+ Repair(f"joins[{k}].right_column_id", join.right_column_id, fixed)
149
+ )
150
+ join.right_column_id = fixed
151
+
152
+ return ir, repairs
153
+
154
+
155
+ # ---------------------------------------------------------------------------
156
+ # Matching helpers
157
+ # ---------------------------------------------------------------------------
158
+
159
+
160
+ def _maybe_fix(value: str, known: list[str]) -> str | None:
161
+ """Return the unique near-miss for `value`, or None if it needs no/ambiguous fix."""
162
+ if value in known:
163
+ return None
164
+ return _unique_near(value, known)
165
+
166
+
167
+ def _unique_near(value: str, known: list[str]) -> str | None:
168
+ """The single catalog id within edit-distance 1 of `value`, else None.
169
+
170
+ Returns None when `value` already matches, when nothing is close, or when
171
+ 2+ candidates are equally close (ambiguous — refuse to guess).
172
+ """
173
+ if value in known:
174
+ return None
175
+ candidates = [k for k in known if _edit_distance_le_1(value, k)]
176
+ return candidates[0] if len(candidates) == 1 else None
177
+
178
+
179
+ def _edit_distance_le_1(a: str, b: str) -> bool:
180
+ """True iff `a` and `b` are within Levenshtein distance 1.
181
+
182
+ Covers the three single-character typos an LLM makes when copying a hash id:
183
+ substitution (same length, one char differs), deletion (a is b with one char
184
+ removed — this is the "dropped the trailing char" case), and insertion (a is
185
+ b with one extra char).
186
+ """
187
+ if a == b:
188
+ return True
189
+ la, lb = len(a), len(b)
190
+ if abs(la - lb) > 1:
191
+ return False
192
+ if la == lb:
193
+ return sum(1 for x, y in zip(a, b, strict=True) if x != y) == 1
194
+ # Lengths differ by exactly 1: check the shorter is the longer minus one char.
195
+ shorter, longer = (a, b) if la < lb else (b, a)
196
+ i = j = 0
197
+ edited = False
198
+ while i < len(shorter) and j < len(longer):
199
+ if shorter[i] == longer[j]:
200
+ i += 1
201
+ j += 1
202
+ elif edited:
203
+ return False
204
+ else:
205
+ edited = True
206
+ j += 1 # consume one char from the longer string
207
+ return True
208
+
209
+
210
+ def _find_source(catalog: Catalog, source_id: str) -> Source | None:
211
+ return next((s for s in catalog.sources if s.source_id == source_id), None)
212
+
213
+
214
+ def _find_table(source: Source, table_id: str) -> Table | None:
215
+ return next((t for t in source.tables if t.table_id == table_id), None)
src/query/service.py CHANGED
@@ -24,6 +24,7 @@ from src.middlewares.logging import get_logger
24
  from ..catalog.models import Catalog
25
  from .executor.base import QueryResult
26
  from .executor.dispatcher import ExecutorDispatcher
 
27
  from .ir.validator import IRValidationError, IRValidator
28
  from .planner.service import QueryPlannerService
29
 
@@ -42,11 +43,13 @@ class QueryService:
42
  self,
43
  planner: QueryPlannerService | None = None,
44
  validator: IRValidator | None = None,
 
45
  dispatcher_factory: Callable[[Catalog], ExecutorDispatcher] | None = None,
46
  max_retries: int = 3,
47
  ) -> None:
48
  self._planner = planner or QueryPlannerService()
49
  self._validator = validator or IRValidator()
 
50
  self._dispatcher_factory = dispatcher_factory or ExecutorDispatcher
51
  self._max_retries = max(1, max_retries)
52
 
@@ -69,6 +72,15 @@ class QueryService:
69
  return _error_result(source_id="", error=f"planner failed: {e}")
70
 
71
  try:
 
 
 
 
 
 
 
 
 
72
  self._validator.validate(ir, catalog)
73
  logger.info(
74
  "ir planned and validated",
 
24
  from ..catalog.models import Catalog
25
  from .executor.base import QueryResult
26
  from .executor.dispatcher import ExecutorDispatcher
27
+ from .ir.repair import IRRepairer
28
  from .ir.validator import IRValidationError, IRValidator
29
  from .planner.service import QueryPlannerService
30
 
 
43
  self,
44
  planner: QueryPlannerService | None = None,
45
  validator: IRValidator | None = None,
46
+ repairer: IRRepairer | None = None,
47
  dispatcher_factory: Callable[[Catalog], ExecutorDispatcher] | None = None,
48
  max_retries: int = 3,
49
  ) -> None:
50
  self._planner = planner or QueryPlannerService()
51
  self._validator = validator or IRValidator()
52
+ self._repairer = repairer or IRRepairer()
53
  self._dispatcher_factory = dispatcher_factory or ExecutorDispatcher
54
  self._max_retries = max(1, max_retries)
55
 
 
72
  return _error_result(source_id="", error=f"planner failed: {e}")
73
 
74
  try:
75
+ ir, repairs = self._repairer.repair(ir, catalog)
76
+ for r in repairs:
77
+ logger.info(
78
+ "repaired ir id",
79
+ attempt=attempt,
80
+ where=r.where,
81
+ from_id=r.from_id,
82
+ to_id=r.to_id,
83
+ )
84
  self._validator.validate(ir, catalog)
85
  logger.info(
86
  "ir planned and validated",
src/tools/data_access.py CHANGED
@@ -36,12 +36,16 @@ from pydantic import ValidationError
36
 
37
  from src.catalog.models import Catalog
38
  from src.catalog.reader import CatalogReader
 
39
  from src.query.executor.dispatcher import ExecutorDispatcher
40
  from src.query.ir.models import QueryIR
 
41
  from src.query.ir.validator import IRValidationError, IRValidator
42
  from src.retrieval.base import RetrievalResult
43
  from src.tools.contracts import ToolOutput
44
 
 
 
45
  DispatcherFactory = Callable[[Catalog], ExecutorDispatcher]
46
 
47
  # Canonical set of data-access tool names — the single source of truth for which
@@ -72,6 +76,7 @@ class DataAccessToolInvoker:
72
  catalog_reader: CatalogReader,
73
  *,
74
  ir_validator: IRValidator | None = None,
 
75
  dispatcher_factory: DispatcherFactory | None = None,
76
  document_retriever: Retriever | None = None,
77
  ) -> None:
@@ -81,6 +86,7 @@ class DataAccessToolInvoker:
81
  # validator is stateless; the dispatcher is built per-call from the
82
  # request's catalog (executors are picked by source_type).
83
  self._validator = ir_validator or IRValidator()
 
84
  self._dispatcher_factory: DispatcherFactory = (
85
  dispatcher_factory or ExecutorDispatcher
86
  )
@@ -223,6 +229,14 @@ class DataAccessToolInvoker:
223
 
224
  catalog = await self._reader.read(self._user_id, "structured")
225
 
 
 
 
 
 
 
 
 
226
  try:
227
  self._validator.validate(ir, catalog)
228
  except IRValidationError as exc:
 
36
 
37
  from src.catalog.models import Catalog
38
  from src.catalog.reader import CatalogReader
39
+ from src.middlewares.logging import get_logger
40
  from src.query.executor.dispatcher import ExecutorDispatcher
41
  from src.query.ir.models import QueryIR
42
+ from src.query.ir.repair import IRRepairer
43
  from src.query.ir.validator import IRValidationError, IRValidator
44
  from src.retrieval.base import RetrievalResult
45
  from src.tools.contracts import ToolOutput
46
 
47
+ logger = get_logger("ir_repair")
48
+
49
  DispatcherFactory = Callable[[Catalog], ExecutorDispatcher]
50
 
51
  # Canonical set of data-access tool names — the single source of truth for which
 
76
  catalog_reader: CatalogReader,
77
  *,
78
  ir_validator: IRValidator | None = None,
79
+ ir_repairer: IRRepairer | None = None,
80
  dispatcher_factory: DispatcherFactory | None = None,
81
  document_retriever: Retriever | None = None,
82
  ) -> None:
 
86
  # validator is stateless; the dispatcher is built per-call from the
87
  # request's catalog (executors are picked by source_type).
88
  self._validator = ir_validator or IRValidator()
89
+ self._repairer = ir_repairer or IRRepairer()
90
  self._dispatcher_factory: DispatcherFactory = (
91
  dispatcher_factory or ExecutorDispatcher
92
  )
 
229
 
230
  catalog = await self._reader.read(self._user_id, "structured")
231
 
232
+ # Repair near-miss ids (an LLM-mangled catalog id) before validating, so a
233
+ # direct retrieve_data call is as resilient as the planner path.
234
+ ir, repairs = self._repairer.repair(ir, catalog)
235
+ for r in repairs:
236
+ logger.info(
237
+ "repaired ir id", where=r.where, from_id=r.from_id, to_id=r.to_id
238
+ )
239
+
240
  try:
241
  self._validator.validate(ir, catalog)
242
  except IRValidationError as exc: