Repository navigation
fix(ai): per-run repeat guard and one session per PR review - #1382
Conversation
review_files workers run the parent's own Maple tool handlers, so the identical-call guard's map was shared across the parent and every worker. Once the parent made three identical pr_changed_files calls, later workers were refused on their first call. The guard now keys on the engine run id from AgentSpawner, which each child run gets with its own identity. A worker's engine invoke_agent span carried no maple_ai.session.id, so ingest fell back to gen_ai.conversation.id (the worker's own thread) and one review showed up as several sessions. runChatTurn now stamps the run's session and turn attributes on every invoke_agent and execute_tool span.
|
Note A newer push replaced |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The reviewed change has no identified merge-blocking issue and is ready for normal checks. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title accurately summarizes both primary changes: scoping the repeat guard per engine run and using one session for each PR review. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 6… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
📝 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.
Comment @coderabbitai help to get the list of available commands.
…ring
A MutableHashMap keyed by { runId, tool, params } uses Effect's structural
Equal/Hash, so there is no string concatenation or JSON.stringify, and the
same arguments in another key order now count as the same call.
Maple review🟢 Confidence 9/10 · safe to merge The identical-call guard in
|
1 participant
What
Two fixes for the PR review agent's
review_filesworkers.1. Workers were refused on their first
pr_changed_files/pr_contextcallThe identical-call guard in
buildMapleToolkitkeeps its counts in a map created once per toolkit build.buildReviewFanouthands the workers the parent's own handlers, so the parent and every worker shared that map. Once the parent made three identical calls, every later worker got "has already been called 3 times" on its first call. In production that was 94 failed calls across 30 sessions in one day.The guard now keys on the engine run id. The engine provides
AgentSpawnerto every run, child runs included, bound to that run's identity. The handler reads it withEffect.serviceOption(AgentSpawner). (Toolkit merges the handler's captured context under the current fiber's, so the calling run's service wins.) Each worker gets its own budget, and the parent's guard is unchanged.The turn id stays shared on purpose: a review is one turn for metering and for turn grouping in the session view. The bug was the shared map, not the shared turn id.
2. One review showed up as several sessions
A worker's model-call and tool spans already carried the parent's
maple_ai.session.id. The engine's owninvoke_agentspan did not, so ingest fell back togen_ai.conversation.id, which is the worker's ownthread-id_*. Tool calls rejected before a handler ran had the same problem.New
withRunSessionAttributestracer wrapper (platform/genai-spans.ts), applied inrunChatTurn, stamps the run's session and turn attributes on everyinvoke_agentandexecute_toolspan in the run. HTTP and database spans are untouched.Tests
chat/run-review-fanout.test.ts(new): a review through the real engine with a scripted model. The parent spends its 3 identical calls, then callsreview_files. The worker's first call must reach the executor, and itsinvoke_agentspan and all tool spans must carry the review's session and turn ids.mcp/tools/llm-tools.test.ts: four sibling runs each get their first call after the parent spent its budget, and the parent is still refused.platform/genai-spans.test.ts: agent and tool spans get the attributes, other spans don't.The two guard tests fail on
mainwith the production symptom and pass here.tscis clean.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit