Skip to content

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

Merged
anandgupta42 merged 4 commits into
mainfrom
refactor/drop-data-qa-agent
Sep 4, 2026
Merged

refactor: drop the data-qa agent, make analyst the data-question agent#1239
anandgupta42 merged 4 commits into
mainfrom
refactor/drop-data-qa-agent

Conversation

@anandgupta42

@anandgupta42 anandgupta42 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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 fix: scope the builder Pre-Execution Protocol to task shape #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. refactor: split builder prompt into invariant core + named packs (byte-identical assembly) + opt-in data-qa profile #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 refactor: split builder prompt into invariant core + named packs (byte-identical assembly) + opt-in data-qa profile #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. refactor: split builder prompt into invariant core + named packs (byte-identical assembly) + opt-in data-qa profile #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 b6392fd. 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.

Written for commit b6392fd. Summary will update on new commits.

Review in cubic

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.

…agent

#1217 added an opt-in `data-qa` agent registered behind
`ALTIMATE_DATA_QA_PROFILE` (prompt = builder-minus-dbt-packs, full
read/write builder permissions). Drop it:

- The name is misleading — "QA" reads as quality-assurance in a data
  tool, not question-answering.
- It overlaps the existing `analyst` agent ("Read-only data exploration
  and analysis. Cannot modify files or run destructive SQL.") while
  keeping FULL read/write builder permissions — a slim-prompt agent
  that can run destructive SQL, the exact safety concern that got
  PR #1215 closed.
- `analyst` already IS the safe read-only data agent; `analyst.txt` is
  already lean (no dbt-build/finish-protocol weight), so the insight
  behind data-qa (dropping build packs is score-neutral on read-only
  data questions) requires no prompt change — analyst already embodies
  it.

Changes:
- `agent/agent.ts`: remove the conditional `data-qa` registration
  block (env flag, config-entry, and `default_agent` opt-ins) and the
  now-unused `Flag` import.
- `altimate/prompts/profiles.ts`: remove `DATA_QA_PROFILE` and
  `PROMPT_DATA_QA`. `BUILDER_PROFILE` / `PROMPT_BUILDER` / `assemble` /
  `FRAGMENTS` — the pack-split architecture from #1217 — are untouched;
  the builder byte-identity pin (sha256 `17663410dd9a…`) stays green.
- `session/termination.ts`: drop `data-qa` from
  `COMPLETION_CONTRACT_AGENTS` (builder stays).
- Delete `test/agent/data-qa-profile.test.ts`; remove the "data-qa
  profile composition" describe block from
  `test/altimate/prompt-profiles.test.ts` (builder byte-identity +
  determinism tests kept); drop the data-qa assertions from
  `test/session/termination.test.ts`.
- Docs: no page ever documented `data-qa`, so nothing to remove there.
  Updated the agent-reference docs (`configure/agents.md`,
  `data-engineering/agent-modes.md`, `getting-started.md`,
  `getting-started/quickstart.md`) to explicitly frame `analyst` as
  the agent for asking questions about your data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_8f76a9b1-b361-488a-93be-cadccf2a56fd)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T01:53:26.613307Z b6392fd New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
- - - - - - - - - - - - - - - - - - - - - - - - -
                    AIRECEIPTS                    
             1 session behind this PR             

claude-sonnet-5..........................≥ $9.1050
  session slice: turns 1–158 of 162
--------------------------------------------------
TOTAL priced.............................≥ $9.1050
  standard API-equivalent floor; not an invoice
  counted: 1 session
  cache served 98% of input tokens
  full receipts + session ids: section below
- - - - - - - - - - - - - - - - - - - - - - - - -
                npx aireceipts-cli                
         github.com/anandgupta42/receipts         
- - - - - - - - - - - - - - - - - - - - - - - - -
full receipts (1 session)
session id scope turns time tokens in / out cached
builder a5a7058a turns 1–158 of 162 158 7h 49m 316 / 4k 98%

builder · a5a7058a

- - - - - - - - - - - - - - - - - - - - - - - - -
                    AIRECEIPTS                    
 “Create a follow-up PR on AltimateAI/altimate-…” 
   Claude Code · Sep 03 2026 17:59 UTC · 7h 49m   
               claude-sonnet-5 100%               
         cache served 98% of input tokens         

pre-edit: 4% of priced floor (6/158 turns)
  (share before the first named edit tool)

Bash........................≥ $6.6854  (108 calls)
Edit.........................≥ $1.0266  (23 calls)
Read.........................≥ $0.5605  (12 calls)
Write.........................≥ $0.5105  (9 calls)
Monitor.......................≥ $0.1676  (3 calls)
(thinking/reply)..............≥ $0.1089  (2 turns)
ToolSearch.....................≥ $0.0452  (1 call)

≈ re-priced eligible trivial spans.......≈ $0.0363
  (2 tiny turns, priced at claude-haiku-4-5)
--------------------------------------------------
TOTAL....................................≥ $9.1047
standard API-equivalent floor; not an invoice
same tokens on claude-haiku-4-5..........≥ $3.0350
  (67% lower observable floor)
  (arithmetic, not a prediction)
- - - - - - - - - - - - - - - - - - - - - - - - -
                npx aireceipts-cli                
         github.com/anandgupta42/receipts         
- - - - - - - - - - - - - - - - - - - - - - - - -
handoff — flagged pattern cost ≈ 358,439 tok
FLAGGED PATTERN COST.................≈ 358,439 tok
  heuristic pattern subtotal · not proven savings

≈ re-priced eligible trivial spans.......≈ $0.0363
  (2 tiny turns, priced at claude-haiku-4-5)
  → route short replies to a cheaper model

covers: 1 session · 158 turns · 1 flagged-pattern line

Generated by aireceipts

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

1 similar comment
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: d6c778e4-288b-49e4-a3d1-3418ab362ab9

📥 Commits

Reviewing files that changed from the base of the PR and between f61cd61 and b6392fd.

📒 Files selected for processing (2)
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/agent/agent.test.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change revises Analyst documentation, migrates stale native data-qa defaults to Analyst, preserves custom data-qa configuration, and limits completion contracts to builder.

Changes

Analyst and data-qa updates

Layer / File(s) Summary
Stale data-qa default migration
packages/opencode/src/agent/agent.ts, packages/opencode/test/agent/agent.test.ts
Missing native data-qa defaults now resolve to analyst with a one-time warning. User-defined agent.data-qa configuration still takes precedence.
Data-qa prompt and completion contract
packages/opencode/src/altimate/prompts/profiles.ts, packages/opencode/src/session/termination.ts, packages/opencode/test/altimate/*
DATA_QA_PROFILE and PROMPT_DATA_QA use three prompt fragments. The completion contract now applies only to builder.
Analyst documentation wording
docs/docs/configure/agents.md, docs/docs/data-engineering/agent-modes.md, docs/docs/getting-started.md, docs/docs/getting-started/quickstart.md
Analyst descriptions now state that the mode answers questions about data while retaining read-only and safe-exploration details.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to b6392

Persisted native data-qa defaults now fall back to the read-only analyst agent while explicitly configured data-qa agents remain unchanged. The documented migration paths are covered, and no merge-blocking risk remains.

Poem

A rabbit checks the agent trail
Old data-qa defaults set sail
Analyst catches questions bright
Builder keeps its contract tight
Clearer docs make paths precise

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 5 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: removing the data-qa agent and making analyst the data-question agent.
Description check ✅ Passed The description is complete and directly covers the issue context, change type, implementation details, migration behavior, verification steps, screenshots status, and checklist. The issue reference i…
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/drop-data-qa-agent

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

Comment thread packages/opencode/test/altimate/prompt-profiles.test.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (2 files)
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/test/agent/agent.test.ts
Previous Review Summaries (2 snapshots, latest commit f61cd61)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit f61cd61)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (3 files)
  • packages/opencode/src/session/termination.ts
  • packages/opencode/test/agent/agent.test.ts
  • packages/opencode/test/altimate/prompt-profiles.test.ts

Previous review (commit 3576a54)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/test/altimate/prompt-profiles.test.ts 40 Redundant single-element loop after removing DATA_QA_PROFILE; can be flattened to a direct uniqueness check on BUILDER_PROFILE
Files Reviewed (11 files)
  • docs/docs/configure/agents.md - 0 issues
  • docs/docs/data-engineering/agent-modes.md - 0 issues
  • docs/docs/getting-started.md - 0 issues
  • docs/docs/getting-started/quickstart.md - 0 issues
  • packages/opencode/src/agent/agent.ts - 0 issues
  • packages/opencode/src/altimate/prompts/profiles.ts - 0 issues
  • packages/opencode/src/session/termination.ts - 0 issues
  • packages/opencode/test/agent/data-qa-profile.test.ts - 0 issues
  • packages/opencode/test/altimate/prompt-identity.ts - 0 issues
  • packages/opencode/test/altimate/prompt-profiles.test.ts - 1 issue
  • packages/opencode/test/session/termination.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4-pro · Input: 57.6K · Output: 11K · Cached: 615.6K

Review guidance: REVIEW.md from base branch main

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3576a54d33

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/opencode/src/agent/agent.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/session/termination.ts">

<violation number="1" location="packages/opencode/src/session/termination.ts:186">
P3: This comment incorrectly claims builder was the only previous carrier, but the previous set also injected the instruction into `data-qa`. Describe the builder path as unchanged and state that the removed `data-qa` path no longer receives it.</violation>
</file>

<file name="packages/opencode/src/agent/agent.ts">

<violation number="1" location="packages/opencode/src/agent/agent.ts:334">
P1: When a legacy `agent.data-qa` entry remains, the generic custom-agent path recreates it as a mode-`all` agent with the default permissive rules. Reject this legacy key or migrate it to `analyst` before generic custom-agent creation so SQL writes retain the intended protection.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/opencode/src/agent/agent.ts
Comment thread packages/opencode/src/session/termination.ts Outdated
- codex/cubic (P1): verified and locked in with a regression test that a
  leftover `agent: {"data-qa": {...}}` config entry — a user upgrading
  from #1217 who never cleaned it out — resolves safely through the
  existing generic custom-agent path (already exercised by "custom agent
  from config creates new agent"): mode "all", `native: false`, no
  resurrected `data-qa` prompt or permissions, no crash. Same generic
  path already covers a stale `default_agent: "data-qa"` with no
  matching `agent.data-qa` entry — it throws the same clear, tested
  "default agent \"data-qa\" not found" error any other stale/renamed
  `default_agent` value produces (see "defaultAgent throws when
  default_agent points to non-existent agent"). No migration code
  needed — both paths were already correct and tested; added
  `test/agent/agent.test.ts` regression coverage for the config-entry
  case specifically.
- kilo (trivial): simplified the now single-element
  `for (const profile of [BUILDER_PROFILE])` loop in
  `test/altimate/prompt-profiles.test.ts` to a direct assertion on
  `BUILDER_PROFILE`.
- cubic (P3): fixed the stale `session/termination.ts` doc comment that
  still referred to "builder and builder-derived agents" — only builder
  carries the completion contract now.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ
@cursor

cursor Bot commented Sep 3, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_68b5a820-f416-4498-9c7c-31ee1fc0d48a)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

3 similar comments
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f61cd61a16

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/opencode/src/agent/agent.ts
anandgupta42 and others added 2 commits September 3, 2026 18:48
…of throwing

Second facet of the P1 review thread on the data-qa removal
(codex `agent.ts:320`, second thread — "Migrate the removed data-qa
default to analyst").

#1217 let `default_agent: "data-qa"` alone — with no matching
`agent.data-qa` config entry — opt into the native data-qa profile.
Removing that registration left `defaultInfo()` throwing
`default agent "data-qa" not found` for anyone whose persisted config
still sets `default_agent: "data-qa"`. Every call site that resolves
the default agent when none is supplied (`session/prompt.ts`, the
session HTTP routes, ACP) depends on `defaultAgent()`/`defaultInfo()`
succeeding, so an upgrading user in this state could no longer start
an ordinary default-agent session — a real strand, not a cosmetic one.

Fix: in `defaultInfo()`, when `default_agent === "data-qa"` and no
agent actually resolves for that name, fall back to `analyst` — the
documented replacement for read-only data questions — with a one-time
warning logged instead of a hard failure. A user who separately
defines their own `agent.data-qa` config entry is unaffected: that
legitimate custom agent resolves first, before the fallback is ever
considered (see the companion test).

This is scoped to the literal removed name "data-qa" — it does not
change the existing, correct behavior for a generic invalid/typo'd
`default_agent` (still throws; see the pre-existing
"defaultAgent throws when default_agent points to non-existent agent"
test, unchanged).

Added two tests to `test/agent/agent.test.ts`:
- `default_agent: "data-qa"` with no matching agent entry resolves to
  `analyst` (not a throw).
- `default_agent: "data-qa"` WITH an explicit `agent.data-qa` config
  entry still resolves to that user-defined agent, not the migration.

`bun run typecheck` clean; `bun test test/agent/` 53/53 pass (plus the
2 new tests); builder byte-identity pin untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ
The marker guard (bun run script/upstream/analyze.ts --markers) flagged
`let agent = agents[c.default_agent]` in agent.ts's defaultInfo() as
unmarked custom code — the previous commit's altimate_change block
started one line too late, after the const->let conversion needed for
the data-qa default-agent migration. Moved the marker start to cover
that line too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VqnuBDGkh1ZT65Ti7e6DHZ
@cursor

cursor Bot commented Sep 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_f435ad1f-a6e0-4ac9-ab29-929bd93e0af8)

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

2 similar comments
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

1 similar comment
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

👋 This PR was automatically closed by our quality checks.

Common reasons:

  • New GitHub account with limited contribution history
  • PR description doesn't meet our guidelines
  • Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b6392fdb3d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +617 to +619
log.warn(
'the "data-qa" agent was removed; defaulting to "analyst" for read-only data questions — set default_agent to override',
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface the migration warning in normal runs

When a user upgrades with default_agent: "data-qa", this warning is sent through the fork-local Log shim, whose output is disabled unless ALTIMATE_PRINT_LOGS or OPENCODE_PRINT_LOGS is explicitly enabled (src/altimate/util/log.ts). Normal CLI and TUI users therefore receive no warning that their configured writable agent was silently replaced by the read-only analyst, and the configuration remains stale, so the silent substitution repeats on later starts. Emit this through a user-visible diagnostic channel or persistently migrate the setting.

Useful? React with 👍 / 👎.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/agent/agent.ts">

<violation number="1" location="packages/opencode/src/agent/agent.ts:618">
P3: The advertised one-time migration warning never reaches default TUI users. `log.warn` goes through the `Log` shim (`src/altimate/util/log.ts`), whose `shouldLog` requires `printEnabled()` — it only writes when `OPENCODE_PRINT_LOGS`/`ALTIMATE_PRINT_LOGS` is set, and that is deliberately OFF for the in-process TUI server. So an upgrading user whose persisted `default_agent: "data-qa"` is silently redirected to `analyst` will not see the notice that the PR description promises ("Fall back to `analyst` ... with a one-time warning"). Consider surfacing the migration through a channel that reaches interactive users (e.g. a session/notification) rather than the print-gated stderr log.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

if (!warnedRemovedDataQaDefault) {
warnedRemovedDataQaDefault = true
log.warn(
'the "data-qa" agent was removed; defaulting to "analyst" for read-only data questions — set default_agent to override',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The advertised one-time migration warning never reaches default TUI users. log.warn goes through the Log shim (src/altimate/util/log.ts), whose shouldLog requires printEnabled() — it only writes when OPENCODE_PRINT_LOGS/ALTIMATE_PRINT_LOGS is set, and that is deliberately OFF for the in-process TUI server. So an upgrading user whose persisted default_agent: "data-qa" is silently redirected to analyst will not see the notice that the PR description promises ("Fall back to analyst ... with a one-time warning"). Consider surfacing the migration through a channel that reaches interactive users (e.g. a session/notification) rather than the print-gated stderr log.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/agent/agent.ts, line 618:

<comment>The advertised one-time migration warning never reaches default TUI users. `log.warn` goes through the `Log` shim (`src/altimate/util/log.ts`), whose `shouldLog` requires `printEnabled()` — it only writes when `OPENCODE_PRINT_LOGS`/`ALTIMATE_PRINT_LOGS` is set, and that is deliberately OFF for the in-process TUI server. So an upgrading user whose persisted `default_agent: "data-qa"` is silently redirected to `analyst` will not see the notice that the PR description promises ("Fall back to `analyst` ... with a one-time warning"). Consider surfacing the migration through a channel that reaches interactive users (e.g. a session/notification) rather than the print-gated stderr log.</comment>

<file context>
@@ -592,7 +599,28 @@ export const layer = Layer.effect(
+              if (!warnedRemovedDataQaDefault) {
+                warnedRemovedDataQaDefault = true
+                log.warn(
+                  'the "data-qa" agent was removed; defaulting to "analyst" for read-only data questions — set default_agent to override',
+                )
+              }
</file context>

@anandgupta42
anandgupta42 merged commit e4d0a18 into main Sep 4, 2026
33 checks passed
@ralphstodomingo

Copy link
Copy Markdown
Contributor

@anandgupta42 heads-up: main has been red on the TypeScript job since this PR's merge commit (e4d0a18e7) — the two commits before it (e6d381714, 1caa234ff) were green, and every commit/PR merge-ref since fails on the same suite:

(fail) SessionCompaction.buildLedger > errored tool calls are recorded as errored and never count as writes
(fail) SessionCompaction.renderLedger > contains verified writes with ISO event time, advisory wording, and unverified-shell note
(fail) SessionCompaction.renderLedger > a recentCalls limit of 0 still renders the writes section
(fail) SessionCompaction.renderLedger > tail-truncates to the token cap, preserving the header and writes section

(e.g. run https://github.com/AltimateAI/altimate-code/actions/runs/33850616707 — an unrelated PR failing only on these.) Since PR checks run on the merge-ref, this is currently blocking every open PR from going green. Could you take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants