Analyze SLayer query-syntax rejections in mini-interact slayer runs - #50
Conversation
…runs Add scripts/analyze_query_syntax_rejections.py: walks the latest slayer *.trajectory.json per (db, iid), pairs each query-tool call with its result by tool_use_id, and classifies every result via one ordered regex taxonomy into dsl (headline) / semantic / gate / infra / other / ok. The DSL bucket is the "agent wrote an invalid SlayerQuery" headline; semantic (DB-execution errors), gate (submit-without-preview), and infra (permission/schema-drift) are tallied for context but excluded. Computes the failure-mode taxonomy, retry-to-recovery chains, and major-contributor-to-failure flags (direct / never_clean / budget_burn), with a --json payload and a CLI text summary. Add notebooks/query_syntax_rejections.ipynb: the reproducible human-readable view (matplotlib plots + pandas tables), including a detailed per-mode breakdown of every DSL failure mode with real example error text and plain-English descriptions sourced from SUBTAG_DESCRIPTIONS. Add an `analysis` pyproject extra (matplotlib/pandas/jupyter/nbformat/ nbconvert) for the notebook tooling, kept out of `all` so runtime installs stay lean. Tests in tests/scripts/test_analyze_query_syntax_rejections.py pin the taxonomy buckets, retry-chain logic, the three contributor flags, latest- trajectory selection, per-sub_tag example capture/dedup, a description- coverage guard, and a drift guard tying the budget literals to harness.ACTION_COSTS. Headline on the 298 latest slayer tasks (4373 query-tool calls): 399 DSL rejections, order_shape ~46%; 85% of retry chains fixed on the first attempt; 0 tasks where syntax was a major contributor (all 44 failures are wrong_result, recovered syntactically, and ran under a high patience budget). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds ChangesQuery Syntax Rejection Analyzer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/analyze_query_syntax_rejections.py (1)
781-784: 💤 Low valueConsider tracking skipped trajectories for observability.
Silently continuing on parse errors could mask data issues (e.g., truncated files, encoding problems). Adding a counter or debug log would help diagnose unexpected results without breaking the best-effort analysis.
+ skipped = 0 for (db, iid), path in sorted(chosen.items()): try: data = json.loads(path.read_text()) except (json.JSONDecodeError, OSError): + skipped += 1 continue per_task.append(analyze_trajectory(data, db=db, iid=iid)) return { "benchmark": benchmark, "mode": mode, "per_task": per_task, "aggregate": aggregate(per_task), + "skipped_files": skipped, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/analyze_query_syntax_rejections.py` around lines 781 - 784, The try-except block that catches json.JSONDecodeError and OSError exceptions when reading and parsing JSON files is silently continuing without any logging or tracking. Add instrumentation to log or count skipped trajectories when these exceptions occur in the exception handler. This could be implemented by adding a debug log statement that includes the file path and exception details before the continue statement, or by maintaining a counter of skipped files. This will help diagnose data issues like truncated or malformed files without breaking the best-effort analysis flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/analyze_query_syntax_rejections.py`:
- Around line 781-784: The try-except block that catches json.JSONDecodeError
and OSError exceptions when reading and parsing JSON files is silently
continuing without any logging or tracking. Add instrumentation to log or count
skipped trajectories when these exceptions occur in the exception handler. This
could be implemented by adding a debug log statement that includes the file path
and exception details before the continue statement, or by maintaining a counter
of skipped files. This will help diagnose data issues like truncated or
malformed files without breaking the best-effort analysis flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83c1fcf8-251b-4758-9fd6-f9662301e36f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
notebooks/query_syntax_rejections.ipynbpyproject.tomlscripts/analyze_query_syntax_rejections.pytests/scripts/test_analyze_query_syntax_rejections.py
…ark-runs Reconciles this branch's DEV-1555 / DEV-1561 + CR r1 unification work with PR #46 (DEV-1550 SLayer compact-by-default), PR #48 (cascade_for_combo), PR #50 (query-syntax-rejection analysis), and DEV-1545 + DEV-1546 prompt additions on origin/main. Conflict resolutions: * ``_shared_otf_prompts.py`` — replaced HEAD's V1 helper section with origin/main's (it carries the new ``_DEDUP_VS_RAW_ROWS``, ``_TABLE_SET_PROBE``, ``_GRADER_ZERO_VS_ONE_DIAGNOSTIC``, ``_SLAYER_TOOLS_BLOCK`` etc. plus DEV-1550 ModelColumn / memory drill- in paragraphs), restored our ``_AFTER_REJECTED_DISCIPLINE`` (DEV-1555 stage-2), and kept our four ``*_V0`` snapshots appended at the bottom. Stripped lingering ``query_nested`` mentions from the V1 helpers so the unified-tool contract holds on both sides. * ``claude_sdk/agent.py`` — kept the unified ``query`` schema (``source_model`` + projection fields OR ``queries`` array; ``required: []``) and the runtime ``source_model XOR queries`` gate. The handler now builds the SlayerQuery JSON internally and forwards it to origin/main's DEV-1546 ``query_impl(query_json: str, …)`` for single-stage, or to ``query_nested_impl(queries=…)`` for the nested-DAG form — matching ``submit_query``'s pattern. Picked up origin/main's ``distinct_dimension_values`` field as an additional schema property so the DEV-1546 dim-only auto-dedup opt-out stays reachable through the unified shape. * ``eval/autopsy.py`` — combined origin/main's 2-attempt corrective- retry loop (sends Pydantic validation errors back as a ``tool_result`` so the model self-fixes the archeology_10 regression) with our DEV-1555 model-aware client (``_build_anthropic_client(model)`` + ``requires_thinking(model)`` thinking mode + auto tool_choice for Moonshot/Kimi) and the JSON-text fallback for third-party endpoints that don't honor forced tool_choice. The retry only fires when the model used the tool (the JSON-text-fallback path has no tool_use_id to bind the corrective ``tool_result`` to). * ``run.py._per_task_timeout_s`` — origin/main flipped the default to ``_DEFAULT_PER_TASK_TIMEOUT_S = 0.0`` (no cap). Adjusted our DEV-1555 grace logic so the runaway grace is only added when the operator explicitly opted in to a positive cap — the default-uncapped contract now holds. * V0 prompts kept as thin re-exports from ``_shared_otf_prompts.V0`` snapshots (origin/main had touched ``claude_sdk_otf*/prompts.py`` with DEV-1545/1550 helper composition, but our side reduced those files to one-line re-exports so the V0 snapshot is the single source of truth for the v0 surface). * ``test_shared_otf_prompts.py`` — re-baselined the V1 SHA pins to ``3fa05ac2…``/``65b8eb05…`` → final post-cleanse ``e671aea3…``/``a3fd695c…`` after stripping lingering ``query_nested`` mentions from the merged V1 helpers. * ``test_dev1534_query_wrapper.py`` — rewrote the schema pin to match the unified shape (``source_model``, ``queries``, ``required: []``); the prior DEV-1546 ``query_json``-only pin is superseded. * ``test_dev1546_distinct_dim_values.py`` — repointed the dedup composition tests at the ``SLAYER_OTF_*_V0`` snapshots (where the full ``_DEDUP_VS_RAW_ROWS`` body is inlined byte-for-byte); the v1 prompts teach the same guidance in a shorter form not assembled via the live constant. Rewrote the query-tool-schema test to assert the unified-shape surface. * ``test_dev1555_query_unified_schema.py`` — updated the mock ``query_impl`` to take ``query_json: str`` positional + kwargs (matches DEV-1546's signature) and parse it to verify the wrapper built the right SlayerQuery dict. Full non-integration suite: 3212 passed, 94 skipped, 50 deselected, 0 failed.
…arch extra Bump the `motley-slayer` floor from 0.7.4 to 0.8.1 and switch the extra from the legacy `embedding-search` to `advanced-search` (the current name; `embedding-search` is now only a graph-less legacy alias). Updated in every reference: both `pyproject.toml` extras, `.mcp.json`, the `harness.py` install-hint message, and the floor-guard regex in `test_dev1546_distinct_dim_values.py`. `uv.lock` relocked (0.8.0 → 0.8.1; adds `ladybug` from advanced-search). 0.8.x fixes the filter-on-aggregate / query-declared-measure / inline- aggregation cases (DEV-1443, DEV-1568) that the query-syntax-rejection analysis (PR #50) found to be the dominant `filter_construction` failure mode in slayer-mode runs — those now compile to `HAVING` instead of being rejected. Full non-integration suite green on 0.8.1: 3220 passed, 94 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Quantifies how often the agent's SlayerQuery DSL payloads get rejected in slayer mode, what it gets wrong, and whether that friction ever sinks a task.
scripts/analyze_query_syntax_rejections.py— importable library + CLI. Selects the latest slayer*.trajectory.jsonper(db, iid), pairs each query-tool call with its result bytool_use_id, and classifies every result via one ordered regex taxonomy intodsl(headline) /semantic/gate/infra/other/ok. Computes the DSL failure-mode taxonomy, retry-to-recovery chains, and major-contributor-to-failure flags (direct/never_clean/budget_burn).--jsonfor the payload, plain text for a terminal summary.notebooks/query_syntax_rejections.ipynb— the reproducible human-readable view (matplotlib plots + pandas tables), including a detailed per-mode breakdown of every DSL failure mode with real example error text + plain-English descriptions.analysispyproject extra (matplotlib/pandas/jupyter/nbformat/nbconvert) for the notebook tooling, kept out ofallso runtime installs stay lean.harness.ACTION_COSTS.Scope
Headline = DSL-validation rejections only (SLayer's query compiler refused the spec). Excluded but tallied for context:
semantic(DB-execution failures —no such column/table, generated-SQLsyntax error),gate(submit_querywithout previewing viaqueryfirst), andinfra(permission-mode denials, schema-drift — harness artefacts).Findings (298 latest slayer tasks, 4373 query-tool calls)
order_shapedominates at ~46% (writingorderas{"col":"desc"}instead of{"column":"col","direction":"desc"}). The non-order_shapetail (~54%, 15 modes) clusters into filter composition, aggregation/measure idioms, and name/shape resolution.wrong_result(semantic miss), the agent always recovered syntactically, and the high patience budget (~1010 coins) meant ≤1-coin retries never came close to exhaustion.Takeaway: DSL friction is a prompt/ergonomics opportunity (fixing the
ordershape alone removes ~46% of rejections), not a driver of failures in these runs.Test plan
env -u SSH_AUTH_SOCK uv run --extra all --extra dev --extra pydantic-ai pytest→ 2795 passed, 94 skipped.uv run python scripts/analyze_query_syntax_rejections.py --mode slayerreproduces the headline numbers above.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
analysisdependency extra with notebook tools for analysis workflows.Tests