feat(wiki-compose): autonomous research loop without user iteration setting - #1055
Conversation
…etting Remove the Brief-phase research depth slider and let the evaluator LLM decide when sources are sufficient. Wiki Compose now uses an internal safety cap instead of a user-configured 1..5 iteration limit; ingest planner keeps its explicit 1..5 cap when provided.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR introduces an explicit ChangesResearch Graph Sufficiency & Exit Conditions
Wiki Compose Brief Resume Payload Simplification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the user-facing "Research depth" slider from the Wiki Compose interface, transitioning the research process to an autonomous loop. The research depth is now determined dynamically by an evaluator LLM using a sufficiency score threshold of 0.75, with a hard safety cap of 10 iterations to prevent runaway loops. Legacy explicit iteration caps (1 to 5) are still supported and resolved appropriately. All corresponding UI components, translation keys, schemas, state variables, and tests have been updated to reflect this change. I have no feedback to provide as the changes are clean and well-tested.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Use graphId instead of a 1..5 numeric heuristic to choose iteration caps, consolidate ingest clamping in constants.ts, add Evaluation.sufficient, and distinguish safety_cap from max_iterations in exit reasons.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
server/api/src/agents/subgraphs/research/shouldRefine.ts (1)
10-21: 💤 Low valueClarify the documentation for
isResearchSufficient.The comment says "Prefers the evaluator's explicit
sufficientflag; falls back to score threshold", but the logic only honorssufficient === trueas a strong signal. Whensufficient === false, the function still falls back to the score threshold, sosufficient: falsedoesn't prevent stopping if the score is high enough.Consider rephrasing to: "Returns true if the evaluator sets
sufficient: true, or if the score meets the threshold" or "Honors explicitsufficient: trueto force stopping; otherwise checks score threshold."This makes it clearer that
sufficient: trueis a one-way override (forces stop) rather thansufficientbeing the primary decision in both directions.🤖 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 `@server/api/src/agents/subgraphs/research/shouldRefine.ts` around lines 10 - 21, Update the JSDoc for isResearchSufficient to clarify that only sufficient === true forces a true return while sufficient === false does not block success — the function returns true if the evaluator sets sufficient: true or if evaluation.score >= RESEARCH_SUFFICIENCY_SCORE_THRESHOLD; reference the Evaluation type, the isResearchSufficient function, and RESEARCH_SUFFICIENCY_SCORE_THRESHOLD in the comment so readers know the explicit-true override and the score fallback behavior.server/api/src/__tests__/agents/subgraphs/research/researchGraph.conditional.test.ts (1)
96-107: ⚡ Quick winConsider adding a test for
sufficient: falsewith high score.The new test on line 96-106 covers
sufficient: truewith low score (forces compile). For completeness, consider adding a test for the opposite edge case:sufficient: falsewith high score (e.g.,score: 0.95).Based on the current logic, this should compile (score threshold wins), but an explicit test would document this behavior clearly.
📝 Suggested test case
it("compiles when score is high even if sufficient is false", () => { expect( shouldRefine( state({ iteration: 1, maxIterations: 5, lastEvaluation: { score: 0.95, sufficient: false, rationale: "conservative", missingAspects: [] }, }), ), ).toBe("compile"); });🤖 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 `@server/api/src/__tests__/agents/subgraphs/research/researchGraph.conditional.test.ts` around lines 96 - 107, Add a new unit test in the same file to assert the edge case where lastEvaluation.sufficient is false but score is above the threshold: create an it("compiles when score is high even if sufficient is false") that calls shouldRefine(state({ iteration: 1, maxIterations: 5, lastEvaluation: { score: 0.95, sufficient: false, rationale: "conservative", missingAspects: [] }})) and expect the result toBe("compile"); this mirrors the existing test pattern and documents that the score threshold overrides sufficient.server/api/src/agents/subgraphs/research/nodes/evaluateSufficiency.ts (1)
29-35: 💤 Low valueConsider clarifying the relationship between
sufficientandscorein the prompt.The prompt instructs the model to set both
sufficient(boolean) andscore(0..1), and mentions ">= 0.75 typically means sufficient". The relationship between these two signals could be made more explicit: should the model set them in agreement, or are they independent? For example, if the model setssufficient: true, should it typically also give a score >= 0.75?Current behavior (based on downstream logic) allows disagreement, which is fine, but the prompt could guide the model on this.
🤖 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 `@server/api/src/agents/subgraphs/research/nodes/evaluateSufficiency.ts` around lines 29 - 35, Clarify in the SYSTEM_PROMPT how `sufficient` and `score` should relate: update the prompt string (the constant SYSTEM_PROMPT that already references RESEARCH_SUFFICIENCY_SCORE_THRESHOLD) to instruct the model to normally make `sufficient: true` when `score` >= ${RESEARCH_SUFFICIENCY_SCORE_THRESHOLD} and `sufficient: false` when `score` < that threshold, but allow the model to override when it provides a short rationale explaining the exception; ensure the wording requires JSON output containing `sufficient`, `score`, `rationale`, and up to 5 `missing_aspects`.server/api/src/agents/subgraphs/research/constants.ts (1)
23-29: ⚡ Quick winConsider importing graph ID from the owner module to eliminate duplication.
The comment on Line 25 asks to keep
INGEST_RESEARCH_GRAPH_IDin sync withINGEST_PLANNER_GRAPH_IDin another file. This string constant duplication creates a maintenance risk—if the values drift,resolveResearchMaxIterationswill silently apply the wrong cap to ingest research runs.A more maintainable approach: have the ingest graph module export its ID constant, then import it here rather than duplicating the string literal.
🤖 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 `@server/api/src/agents/subgraphs/research/constants.ts` around lines 23 - 29, The constant INGEST_RESEARCH_GRAPH_ID duplicates the string from INGEST_PLANNER_GRAPH_ID and risks drifting; instead import the canonical graph id exported by the ingest planner module (the symbol INGEST_PLANNER_GRAPH_ID) and re-export or alias it here (replace the local INGEST_RESEARCH_GRAPH_ID definition with an import from the ingest/ingestPlannerGraph module and, if needed for API stability, export it under the same name so resolveResearchMaxIterations uses the single source of truth).server/api/src/__tests__/agents/subgraphs/research/constants.test.ts (1)
13-19: ⚡ Quick winAdd boundary and edge case tests for more comprehensive validation.
The current tests cover the happy path, but adding explicit boundary cases would improve confidence in the clamping logic:
- Boundary values:
0 → 1,1 → 1,5 → 5,6 → 5,-1 → 1- Fractional inputs:
1.5 → 1,4.7 → 4,5.9 → 5- Already handled but worth explicit tests:
null,NaN,"3"The implementation handles these correctly via
typeof/isFinite/truncchecks, but explicit test cases document expected behavior and catch future regressions.🧪 Suggested additional test cases
describe("clampIngestMaxIterations", () => { it("clamps ingest caps to 1..5 with default 3", () => { expect(clampIngestMaxIterations(undefined)).toBe(3); expect(clampIngestMaxIterations(99)).toBe(5); expect(clampIngestMaxIterations(4)).toBe(4); }); + + it("handles boundary values", () => { + expect(clampIngestMaxIterations(0)).toBe(1); + expect(clampIngestMaxIterations(1)).toBe(1); + expect(clampIngestMaxIterations(5)).toBe(5); + expect(clampIngestMaxIterations(6)).toBe(5); + expect(clampIngestMaxIterations(-1)).toBe(1); + }); + + it("truncates fractional inputs", () => { + expect(clampIngestMaxIterations(1.5)).toBe(1); + expect(clampIngestMaxIterations(4.7)).toBe(4); + expect(clampIngestMaxIterations(5.9)).toBe(5); + }); + + it("rejects non-numeric inputs with default", () => { + expect(clampIngestMaxIterations(null)).toBe(3); + expect(clampIngestMaxIterations(NaN)).toBe(3); + expect(clampIngestMaxIterations("3")).toBe(3); + }); });🤖 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 `@server/api/src/__tests__/agents/subgraphs/research/constants.test.ts` around lines 13 - 19, The test suite for clampIngestMaxIterations is missing explicit boundary and edge-case assertions; add unit tests in constants.test.ts calling clampIngestMaxIterations for 0,1,5,6,-1 and expect 1,1,5,5,1 respectively, add fractional inputs like 1.5→1, 4.7→4, 5.9→5, and add null, NaN, and string inputs (e.g., null, NaN, "3") to assert the function’s current behavior (undefined-like default to 3 or appropriate clamped/truncated results) so these edge cases are documented and will catch regressions.
🤖 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 `@server/api/src/__tests__/agents/subgraphs/research/constants.test.ts`:
- Around line 13-19: The test suite for clampIngestMaxIterations is missing
explicit boundary and edge-case assertions; add unit tests in constants.test.ts
calling clampIngestMaxIterations for 0,1,5,6,-1 and expect 1,1,5,5,1
respectively, add fractional inputs like 1.5→1, 4.7→4, 5.9→5, and add null, NaN,
and string inputs (e.g., null, NaN, "3") to assert the function’s current
behavior (undefined-like default to 3 or appropriate clamped/truncated results)
so these edge cases are documented and will catch regressions.
In
`@server/api/src/__tests__/agents/subgraphs/research/researchGraph.conditional.test.ts`:
- Around line 96-107: Add a new unit test in the same file to assert the edge
case where lastEvaluation.sufficient is false but score is above the threshold:
create an it("compiles when score is high even if sufficient is false") that
calls shouldRefine(state({ iteration: 1, maxIterations: 5, lastEvaluation: {
score: 0.95, sufficient: false, rationale: "conservative", missingAspects: []
}})) and expect the result toBe("compile"); this mirrors the existing test
pattern and documents that the score threshold overrides sufficient.
In `@server/api/src/agents/subgraphs/research/constants.ts`:
- Around line 23-29: The constant INGEST_RESEARCH_GRAPH_ID duplicates the string
from INGEST_PLANNER_GRAPH_ID and risks drifting; instead import the canonical
graph id exported by the ingest planner module (the symbol
INGEST_PLANNER_GRAPH_ID) and re-export or alias it here (replace the local
INGEST_RESEARCH_GRAPH_ID definition with an import from the
ingest/ingestPlannerGraph module and, if needed for API stability, export it
under the same name so resolveResearchMaxIterations uses the single source of
truth).
In `@server/api/src/agents/subgraphs/research/nodes/evaluateSufficiency.ts`:
- Around line 29-35: Clarify in the SYSTEM_PROMPT how `sufficient` and `score`
should relate: update the prompt string (the constant SYSTEM_PROMPT that already
references RESEARCH_SUFFICIENCY_SCORE_THRESHOLD) to instruct the model to
normally make `sufficient: true` when `score` >=
${RESEARCH_SUFFICIENCY_SCORE_THRESHOLD} and `sufficient: false` when `score` <
that threshold, but allow the model to override when it provides a short
rationale explaining the exception; ensure the wording requires JSON output
containing `sufficient`, `score`, `rationale`, and up to 5 `missing_aspects`.
In `@server/api/src/agents/subgraphs/research/shouldRefine.ts`:
- Around line 10-21: Update the JSDoc for isResearchSufficient to clarify that
only sufficient === true forces a true return while sufficient === false does
not block success — the function returns true if the evaluator sets sufficient:
true or if evaluation.score >= RESEARCH_SUFFICIENCY_SCORE_THRESHOLD; reference
the Evaluation type, the isResearchSufficient function, and
RESEARCH_SUFFICIENCY_SCORE_THRESHOLD in the comment so readers know the
explicit-true override and the score fallback behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72ed7f21-9bbd-496e-a25c-0eff045ecc47
📒 Files selected for processing (37)
server/api/src/__tests__/agents/graphs/ingest/formatResearchForIngest.test.tsserver/api/src/__tests__/agents/graphs/ingest/ingestPlannerGraph.test.tsserver/api/src/__tests__/agents/graphs/wikiCompose/nodes/humanReviewBrief.test.tsserver/api/src/__tests__/agents/graphs/wikiCompose/wikiComposeGraph.test.tsserver/api/src/__tests__/agents/subgraphs/research/constants.test.tsserver/api/src/__tests__/agents/subgraphs/research/nodes/compileBatch.test.tsserver/api/src/__tests__/agents/subgraphs/research/nodes/planQueries.test.tsserver/api/src/__tests__/agents/subgraphs/research/researchGraph.conditional.test.tsserver/api/src/__tests__/agents/subgraphs/research/researchGraph.interrupt.test.tsserver/api/src/__tests__/agents/subgraphs/research/researchGraph.loop.test.tsserver/api/src/__tests__/agents/subgraphs/research/researchGraph.modelGuard.test.tsserver/api/src/__tests__/agents/subgraphs/research/researchGraph.resume.test.tsserver/api/src/agents/core/types/sseEvents.tsserver/api/src/agents/graphs/ingest/nodes/prepareIngest.tsserver/api/src/agents/graphs/wikiCompose/nodes/humanReviewBrief.tsserver/api/src/agents/graphs/wikiCompose/resumeSchemas.tsserver/api/src/agents/graphs/wikiCompose/state.tsserver/api/src/agents/graphs/wikiCompose/types.tsserver/api/src/agents/runner/sseMapper.tsserver/api/src/agents/subgraphs/research/constants.tsserver/api/src/agents/subgraphs/research/nodes/compileBatch.tsserver/api/src/agents/subgraphs/research/nodes/evaluateSufficiency.tsserver/api/src/agents/subgraphs/research/nodes/planQueries.tsserver/api/src/agents/subgraphs/research/nodes/refineQueries.tsserver/api/src/agents/subgraphs/research/nodes/shared/dispatchSseCustom.tsserver/api/src/agents/subgraphs/research/researchGraph.tsserver/api/src/agents/subgraphs/research/shouldRefine.tsserver/api/src/agents/subgraphs/research/state.tsserver/api/src/agents/subgraphs/research/types.tsserver/api/src/routes/composeSessions.tssrc/components/wikiCompose/ComposePanel.tsxsrc/components/wikiCompose/DialogueSection.tsxsrc/hooks/wiki/useWikiComposeSession.tssrc/i18n/locales/en/wikiCompose.jsonsrc/i18n/locales/ja/wikiCompose.jsonsrc/lib/wikiCompose/composeService.tssrc/lib/wikiCompose/types.ts
💤 Files with no reviewable changes (4)
- src/i18n/locales/en/wikiCompose.json
- src/i18n/locales/ja/wikiCompose.json
- server/api/src/tests/agents/graphs/wikiCompose/nodes/humanReviewBrief.test.ts
- server/api/src/agents/graphs/wikiCompose/types.ts
Introduce ResearchLoopCompileExitReason so compile_batch dispatch matches the SSE payload union and fixes API Type Check.
2 participants
概要
Wiki Compose の Brief フェーズにあった「調査の深さ（1〜5 回）」スライダーを廃止し、調査ループの終了を評価 LLM の自律判断に委ねるようにしました。コードレビュー指摘に基づき、cap 解決は
graphId分岐に統一し、legacy checkpoint でも誤って 3 回 cap されないようにしています。変更点
server/api/（調査ループ）researchMaxIterationsを削除し、UI/API から回数設定を排除resolveResearchMaxIterations(graphId, …)— ingest のみ state cap (1..5)、Wiki Compose は常に安全上限 (10)Evaluation.sufficientを追加し、shouldRefine/compileBatchが明示フラグを優先exitReason: "safety_cap"を追加（自律モードの安全上限到達をmax_iterationsと区別）RESEARCH_SUFFICIENCY_SCORE_THRESHOLDと ingest clamp をconstants.tsに集約src/（UI）変更の種類
テスト方法
sufficient: trueまたは score ≥ 0.75 で HITL へ進むことmaxIterations: 3でも Wiki Compose が 10 回まで調査できること（planQueries/constantsテストで担保）チェックリスト
関連 Issue
Summary by CodeRabbit
New Features
Bug Fixes
Refactor