refactor: drop the data-qa agent, make analyst the data-question agent

Merged
AN
2mo ago
+130-240

Issue for this PR

Closes #

Follow-up to #1217 (merged to main), acting on the post-merge review decision to drop the data-qa agent it introduced.

Type of change

  • Refactor / code improvement
  • Documentation

What does this PR do?

Removes the opt-in data-qa agent added in #1217 and makes the existing analyst agent the documented data-question agent instead.

Why drop data-qa:

  • Misleading name. In a data tool, "QA" reads as quality-assurance, not question-answering.
  • Overlaps analyst, but unsafely. analyst is already described as "Read-only data exploration and analysis. Cannot modify files or run destructive SQL." data-qa covered the same use case (asking questions about data) but shipped with full read/write builder permissions — a slim-prompt agent that can still run destructive SQL. That is the exact safety concern that got PR #1215 closed. There's no reason to carry a second, less-safe path to the same job analyst already does safely.
  • The insight behind it needed no new agent. #1217's basis for data-qa was an internal ablation showing that dropping the dbt-build packs is score-neutral on read-only data-QA workloads. I read analyst.txt (packages/opencode/src/altimate/prompts/analyst.txt) to check whether it needed the same trim — it's already lean: no dbt build/finish-protocol content, no Pre-Execution Protocol pack, nothing the ablation targeted. analyst already embodies the insight. I left analyst.txt unchanged — the safe default the task called for when the prompt has no removable weight.

What's kept: #1217's real contribution — splitting the monolithic builder.txt into an invariant core.txt + named packs under builder/packs/, assembled byte-identically by profiles.ts — is untouched. BUILDER_PROFILE, PROMPT_BUILDER, assemble, and FRAGMENTS are unmodified, and the builder byte-identity pin (sha256 17663410dd9a… in test/altimate/prompt-identity.ts) stays green.

Changes:

  • packages/opencode/src/agent/agent.ts — removed the conditional data-qa registration block (the ALTIMATE_DATA_QA_PROFILE env flag, cfg.agent["data-qa"] config-entry opt-in, and cfg.default_agent === "data-qa" gate), and the now-unused Flag import.
  • packages/opencode/src/altimate/prompts/profiles.ts — removed DATA_QA_PROFILE and PROMPT_DATA_QA. BUILDER_PROFILE / PROMPT_BUILDER / assemble / FRAGMENTS are untouched.
  • packages/opencode/src/session/termination.ts — removed data-qa from COMPLETION_CONTRACT_AGENTS (now {"builder"} only).
  • Deleted packages/opencode/test/agent/data-qa-profile.test.ts.
  • packages/opencode/test/altimate/prompt-profiles.test.ts — removed the "data-qa profile composition" describe block and the DATA_QA_PROFILE/PROMPT_DATA_QA imports; kept the builder byte-identity and cross-process determinism tests.
  • packages/opencode/test/session/termination.test.ts — removed the data-qa assertions from the completion-contract test.
  • packages/opencode/test/altimate/prompt-identity.ts — updated a stale doc comment referencing the deleted test file.
  • Docs — no page ever documented data-qa or ALTIMATE_DATA_QA_PROFILE (that landed only in code in #1217), so there was nothing to remove. Updated docs/docs/configure/agents.md, docs/docs/data-engineering/agent-modes.md, docs/docs/getting-started.md, and docs/docs/getting-started/quickstart.md to explicitly describe analyst as the agent for asking questions about your data (in addition to its existing "read-only exploration" framing), so the safe choice for "just ask questions about my data" is unambiguous. builder (full read/write) and plan (planning-only) descriptions were left as-is; they were already accurate.

A repo-wide grep for data-qa, DATA_QA, ALTIMATE_DATA_QA_PROFILE, and PROMPT_DATA_QA after these changes returns no results.

Review-round follow-up (upgrade-safety on the two legacy-config paths): codex and cubic flagged what happens to a config that still references the removed agent after an upgrade. Traced both paths and handled each on its actual behavior:

  • Leftover agent: {"data-qa": {...}} config entry — traced the config→agent registration path in agent.ts: it falls through to the existing generic custom-agent path (already covered by "custom agent from config creates new agent"), producing a fully-formed, non-crashing agent (mode: "all", native: false). No migration code needed — verified with a new regression test ("a leftover config agent.data-qa entry resolves as a harmless generic custom agent, not a crash") rather than adding a special case for a key that was never in a release.
  • Persisted default_agent: "data-qa" with no matching agent.data-qa entry — this one did need a fix. #1217 explicitly special-cased this exact configuration to register the native profile, so defaultInfo() would otherwise throw default agent "data-qa" not found and strand every call site that resolves the default agent (session/prompt.ts, session HTTP routes, ACP). Fixed by falling back to analyst — the documented replacement — with a one-time warning, scoped to the literal name "data-qa" (a generic invalid/typo'd default_agent still throws, unchanged). Two new tests cover the fallback and the case where a user's own agent.data-qa entry should win over the migration.

How did you verify your code works?

  • bun run typecheck (repo root, via turbo) — clean.
  • bun test test/agent/ test/altimate/ test/session/ (packages/opencode) — 6119 pass, 0 fail from this change. Two test/session/prompt.test.ts tests (loop calls LLM and returns assistant message, loop surfaces content-filter finishes as session errors) are flaky/timeout-prone against the mocked LLM harness and reference agent "build" (the pre-existing builder alias) — nothing to do with data-qa; the adjacent test in that file already carries a standing BUG: REGRESSION comment about this exact flakiness. Reproduces identically before and after this change.
  • bun test test/altimate/prompt-profiles.test.ts specifically — 4/4 pass, including the builder byte-identity assertion against the pinned sha256 17663410dd9a….
  • bun run script/upstream/analyze.ts --markers --base origin/main --strict — clean (All custom code in upstream-shared files is properly marked); removing the data-qa block shrank the marked altimate_change region in agent.ts, it did not need a new one.
  • Repo-wide grep for data-qa/DATA_QA/ALTIMATE_DATA_QA_PROFILE/PROMPT_DATA_QA — zero remaining references.
  • Follow-up commits (f61cd61a16, e3e47bc6ca, b6392fdb3d) address the review round above: bun test test/agent/ test/altimate/prompt-profiles.test.ts test/altimate/prompt-identity.ts test/session/termination.test.ts — 92/93 pass, 1 pre-existing todo, 0 fail (includes the 3 new regression tests for the legacy-config paths); bun run typecheck clean; bun run script/upstream/analyze.ts --markers --base origin/main --strict clean.
  • CI: pushed and watching for settle before requesting review; will not merge/approve this PR myself.

Screenshots / recordings

N/A — no UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

[!NOTE]
Medium Risk
Changes default-agent resolution for users who still have default_agent: "data-qa" in config; migration to analyst is intentional and tested, but it alters which permissions and prompt users get on upgrade.

Overview
Removes the opt-in data-qa native agent (env flag, config registration, and slim PROMPT_DATA_QA profile) and positions analyst as the supported read-only path for asking questions about data.

Runtime behavior: default_agent: "data-qa" with no matching agent.data-qa block now resolves to analyst with a one-time log warning instead of failing startup; a leftover agent.data-qa config entry becomes a generic custom agent. Run-mode DONE completion instructions apply only to builder ( data-qa dropped from the completion contract).

Docs: Agent and getting-started pages now describe Analyst explicitly for data Q&A. Builder prompt assembly and byte-identity pins are unchanged.

Tests: Deletes data-qa-profile tests and adds regressions for the migration paths above.

Reviewed by Cursor Bugbot for commit b6392fdb3de256b291a0bb2565917999c1a8634d. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by cubic

Removes the opt-in data-qa agent and makes the existing analyst agent the documented agent for asking questions about your data.

data-qa duplicated analyst's purpose while shipping with full read/write builder permissions, and its insight was already embodied in analyst's lean prompt. The builder prompt split into core + named packs is untouched.

Changes

  • Drops the data-qa registration, its env-flag/config opt-ins, DATA_QA_PROFILE, and the completion-contract entry.
  • A persisted default_agent: "data-qa" now resolves to analyst with a one-time warning instead of stranding default-agent sessions; an explicit agent.data-qa config entry still wins.
  • A leftover agent.data-qa config entry resolves as a harmless generic custom agent.
  • Updates docs to describe analyst as the agent for asking questions about your data.

Summary by CodeRabbit

  • Documentation

    • Clarified that Analyst mode supports asking questions about data, safe exploration, and analysis while preventing SQL writes.
    • Updated Analyst descriptions across agent configuration and getting-started guides.

  • Changes

    • Removed the built-in opt-in data-qa agent profile and its associated completion behavior.
    • The builder agent remains responsible for run-mode completion instructions.
    • Persisted configurations referencing data-qa now fall back to Analyst, while user-defined data-qa configurations continue to take precedence.