Repository navigation
Conversation
…pace skills In a linked project the read-only analyst agent could not call any memory tool, yet the workspace identity section told it to save with `altimate_memory_write` and check `altimate_memory_read`. Its prompt named only the built-in data skills and required every reply to finish through `answer_commit`, which suits data values, not a list of conventions. - analyst may call `altimate_memory_read`, `altimate_memory_refresh` and `altimate_memory_audit`; write, delete and extract stay denied, and data-explorer is unchanged. - The team-memory line names only the memory tools the session's agent can call: a read-only agent is pointed at the read tool and told that builder saves; an agent with neither gets no line. - The analyst prompt loads a matching skill by source precedence, answers questions about team conventions in plain text from skills and memory, and keeps `answer_commit` for data questions. Fixes #1569 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. (2 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: enabling Analyst to read workspace memory and use workspace skills. Linked Issues check Issue #1569requirements are implemented.analystallowsaltimate_memory_read,altimate_memory_refresh, andaltimate_memory_audit; its default rules deny memory write, delete, and extract oper…Out of Scope Changes check The changes stay within Issue #1569. The empty-memory response removes guidance to an unavailable write tool. The sharedLLM.removedToolslogic keeps identity text aligned with agent and request too…Description check The pull request follows the required template and provides the issue, change type, implementation details, verification results, cost impact, screenshots status, and completed checklist. It also clea… Full details: Linked Issues check
-
Explanation
Issue
#1569requirements are implemented.analystallowsaltimate_memory_read,altimate_memory_refresh, andaltimate_memory_audit; its default rules deny memory write, delete, and extract operations.data-explorerkeeps its existing read-only memory policy. The prompt loads a matching skill from the available skill list, including workspace and project skills, answers non-data workspace questions in plain text, and keepsanswer_commitfor data questions. Workspace identity text uses the tools retained for the current agent and request. Tests cover these permission, prompt, identity, and tool-filtering behaviors.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within Issue
#1569. The empty-memory response removes guidance to an unavailable write tool. The sharedLLM.removedToolslogic keeps identity text aligned with agent and request tool filtering. Tests and analyst documentation support the requested behavior. No unrelated product change is established by the reviewed summary or focused source evidence.
Full details: Description check
-
Explanation
The pull request follows the required template and provides the issue, change type, implementation details, verification results, cost impact, screenshots status, and completed checklist. It also clearly documents the untested live analyst run and its provider limitation.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit reads each line,
The patch grows clear beneath the moon,
Small changes hop in place,
Tests guard the garden path,
Reviews bloom before the dawn.
Comment @coderabbitai help to get the list of available commands.
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…tool analyst can now call `altimate_memory_read` but not `altimate_memory_write`. An empty read told it to "Use altimate_memory_write to save information", steering a read-only agent toward a call it cannot make. The result now only says that no blocks were found. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
…ization rule The analyst prompt now limits answer_commit to data questions and lets workspace-knowledge answers finish in plain text; update the pinned finalization assertion and pin the new exclusion sentence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Share one removal rule (`LLM.removedTools`) between tool resolution and the workspace identity section, so a memory tool switched off on the request, or not registered, is not named in the prompt. - A write-only agent gets the save line without the read-first step. - The analyst prompt checks `altimate_memory_read` only when it has it. - Update the analyst prompt assertion in sql-validation-e2e to the new finalization wording. - Cover all four read/write combinations and one cached outcome rendered for different agents. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… wiring - The analyst permission comment now says a reload syncs like the load every session runs and can drop a local copy of an archived block, rather than calling it read-only. - The prompt comment scopes the memory line to the tools the agent and request keep; tool-free steps withhold every tool the prompt names. - A prompt-loop test checks that the identity section is given the same memory access the tools handed to the model call imply. - Document analyst's memory and workspace-skill access. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Match the agent-modes page, and name the prompt-loop memory tests by what they check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (16 files)
Previous Review Summary (commit fb215f8)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit fb215f8)Status: No Issues Found | Recommendation: Merge Files Reviewed (16 files)
Reviewed by gemini-4-argon · Input: 61.4K · Output: 1.1K · Cached: 145.3K Review guidance: REVIEW.md from base branch |
There was a problem hiding this comment.
3 issues found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/opencode/test/session/prompt.test.ts">
<violation number="1" location="packages/opencode/test/session/prompt.test.ts:635">
P3: This test never checks the rendered memory text, because `systemSection` is mocked to return "". It only checks the flags passed in. Rename it to describe what it asserts, or add a no-access turn (both tools denied) so the loop covers the case the PR description lists.</violation>
</file>
<file name="packages/opencode/src/agent/agent.ts">
<violation number="1" location="packages/opencode/src/agent/agent.ts:343">
P2: `altimate_memory_audit` reads only this checkout's local `.log`, so it cannot show teammate changes made directly in workspace memory. Add a workspace-aware audit source before exposing it as workspace auditing.</violation>
</file>
<file name="packages/opencode/src/session/prompt.ts">
<violation number="1" location="packages/opencode/src/session/prompt.ts:1923">
P2: `tools` is only the pre-stream catalog: retrieval can still remove either memory tool, so this line can advertise an action the model cannot call. Derive these flags from the final tool set or ensure the named memory tools survive retrieval.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Add an analyst turn with no memory tools to the prompt-loop test and name the test for what it asserts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Multi-model consensus reviewVerdict: approve, with minor follow-ups. No critical or major issues. Five independent reviewers converged in one round, all approving. Read-only check. The three new analyst allows ( Minor
Nits
Done well
Multi-model consensus review (5 reviewers, 1 convergence round, unanimous). |
🤖 Code Review — OpenCodeReview (Gemini) — No Issues FoundNo supported files changed. |
Issue for this PR
Closes #1569
Type of change
What does this PR do?
In a linked project, analyst could not answer read-only questions like "what are our conventions?": every memory tool was denied, the identity section named a save tool it lacked, and every reply had to go through answer_commit.
Small extras beyond the issue: the write-only case is reachable only through user config, and the docs lines.
Before and after
answer_commit, often with no useful answeranswer_commitanswer_commitFollow-ups (existing behaviour, not changed here)
Both issues below came up in review. Each was reported in the issue tracker as a separate bug for follow-up, and neither is fixed in this PR.
altimate_memory_auditreads only this checkout's local audit log. It does not show changes teammates made directly in workspace memory.ALTIMATE_TOOL_RETRIEVAL=1, off by default), retrieval can drop a memory tool after the team-memory line has named it. Before this PR the line named both tools for every agent.How did you verify your code works?
Cost side effects
None. About 110 more tokens in the analyst system prompt and three more tool definitions. No new model calls, queries or resources that grow with user data.
Screenshots / recordings
N/A, no UI change.
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit