sofhiaazzhr Claude Opus 4.8 commited on
Commit
776fae3
·
1 Parent(s): 0772863

fix(planner): reject inlined table-ref as analyze_* data (Pattern A)

Browse files

Check 9 only guarded `data`/`data_right` when it was a string placeholder, so
when the planner inlined a {table_id, source_id} dict (skipping retrieve_data)
the plan slipped through validation and only failed at execution with the opaque
"unsupported 'data' type: dict" — surfaced to the user as a misleading "data type
not supported" answer. Now any non-placeholder `data`/`data_right` is rejected in
the validator so the retry loop re-prompts the planner to chain a retrieve_data
output. Also harden _materialize: a {table_id/source_id} dict returns an
actionable Pattern-A error instead of the bare type name, as a defensive net.

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

src/agents/planner/validator.py CHANGED
@@ -212,23 +212,45 @@ class PlannerValidator:
212
  def _validate_data_source(
213
  task_id: str, call, tasks_by_id: dict, registry: ToolRegistry
214
  ) -> None:
215
- """A `data` placeholder must reference a data-producing task, not a
216
- metadata (check_data/check_knowledge) or documents (retrieve_knowledge) one.
217
-
218
- Those pass the structural checks (check_* also returns kind="table"), but
219
- their rows are catalog schema, so a downstream analyze_* fails to find the
220
- requested columns. Resolving points at the referenced task's representative
221
- output its last tool call (matches TaskRunner's `outputs[-1]`).
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
222
  """
223
- # `data_right` is analyze_merge's second table input (KM-703) — same
224
- # Pattern A handoff, so it gets the same guard.
225
  for arg_name in ("data", "data_right"):
226
- data_arg = call.args.get(arg_name)
227
- if not isinstance(data_arg, str):
228
- continue
229
- match = PLACEHOLDER_RE.fullmatch(data_arg.strip())
230
- if not match:
231
  continue
 
 
 
 
 
 
 
 
 
 
 
 
 
232
  ref_task = tasks_by_id.get(match.group(1))
233
  if ref_task is None or not ref_task.tool_calls:
234
  continue # a dangling placeholder is reported by the DAG check
 
212
  def _validate_data_source(
213
  task_id: str, call, tasks_by_id: dict, registry: ToolRegistry
214
  ) -> None:
215
+ """The `data`/`data_right` handoff (Pattern A) must be a '${t<id>}'
216
+ placeholder pointing at an upstream data-producing task.
217
+
218
+ Two failure modes are rejected here:
219
+
220
+ 1. The arg is present but is NOT a placeholder — e.g. the planner inlined a
221
+ {table_id, source_id} table reference (or any literal) instead of
222
+ chaining a retrieve_data output. analyze_* tools never self-fetch by
223
+ source_id; they only consume an already-retrieved table, so such a plan
224
+ can only blow up at execution ("unsupported 'data' type: dict"). Reject
225
+ it here so the retry loop re-prompts the planner to add a retrieve_data
226
+ task and pass its placeholder.
227
+ 2. The placeholder resolves to a metadata (check_data/check_knowledge) or
228
+ documents (retrieve_knowledge) task. Those pass the structural checks
229
+ (check_* also returns kind="table"), but their rows are catalog schema,
230
+ so a downstream analyze_* fails to find the requested columns. Resolving
231
+ points at the referenced task's representative output — its last tool
232
+ call (matches TaskRunner's `outputs[-1]`).
233
+
234
+ `data_right` is analyze_merge's second table input (KM-703) — same Pattern A
235
+ handoff, so it gets the same guard. An arg that is absent is left to Check 8a
236
+ (required-arg presence).
237
  """
 
 
238
  for arg_name in ("data", "data_right"):
239
+ if arg_name not in call.args:
 
 
 
 
240
  continue
241
+ data_arg = call.args[arg_name]
242
+ match = (
243
+ PLACEHOLDER_RE.fullmatch(data_arg.strip())
244
+ if isinstance(data_arg, str)
245
+ else None
246
+ )
247
+ if match is None:
248
+ raise PlannerValidationError(
249
+ f"task {task_id}: tool {call.tool!r} arg {arg_name!r} must be a "
250
+ f"'${{t<id>}}' placeholder referencing a retrieve_data output "
251
+ f"(Pattern A), got {data_arg!r}. analyze_* tools do not fetch data "
252
+ "themselves — add a retrieve_data task and pass its output."
253
+ )
254
  ref_task = tasks_by_id.get(match.group(1))
255
  if ref_task is None or not ref_task.tool_calls:
256
  continue # a dangling placeholder is reported by the DAG check
src/tools/invoker.py CHANGED
@@ -163,6 +163,18 @@ def _materialize(data: Any) -> tuple[pd.DataFrame, None] | tuple[None, str]:
163
  df = pd.DataFrame(data.get("rows") or [], columns=data["columns"])
164
  return _normalize_numeric(df), None
165
 
 
 
 
 
 
 
 
 
 
 
 
 
166
  return None, f"unsupported 'data' type: {type(data).__name__}"
167
 
168
 
 
163
  df = pd.DataFrame(data.get("rows") or [], columns=data["columns"])
164
  return _normalize_numeric(df), None
165
 
166
+ # A {table_id/source_id} dict is a raw catalog reference the planner inlined
167
+ # instead of chaining a retrieve_data output (Pattern A). analyze_* tools never
168
+ # self-fetch, so give an actionable message rather than the opaque type name.
169
+ # The planner validator should reject this upstream (Check 9); this is the
170
+ # defensive net if a bad plan still reaches execution.
171
+ if isinstance(data, dict) and ("table_id" in data or "source_id" in data):
172
+ return None, (
173
+ "'data' is a table reference (table_id/source_id), not a retrieved "
174
+ "table — analyze_* consumes the output of retrieve_data, it does not "
175
+ "fetch data by id (Pattern A)"
176
+ )
177
+
178
  return None, f"unsupported 'data' type: {type(data).__name__}"
179
 
180