Conversation
Aayushjshah
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
📝 WalkthroughWalkthroughThis PR introduces a centralized Final Answer Synthesis module that orchestrates end-to-end answer generation with token budgeting, section planning, fragment mapping, and streaming output. The message-agents flow is refactored to delegate synthesis to this new module. Model-specific input token limits are added to the configuration system. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MessageAgent as Message<br/>Agent
participant FinalSynthesis as Final Answer<br/>Synthesis
participant Planner
participant Mapper
participant Model
Client->>MessageAgent: Request final answer
MessageAgent->>FinalSynthesis: executeFinalSynthesis(context)
FinalSynthesis->>FinalSynthesis: Estimate input tokens<br/>and select mode
alt Sectional Mode
FinalSynthesis->>Planner: planSections(fragments,<br/>budget, clarifications)
Planner-->>FinalSynthesis: section plan
FinalSynthesis->>FinalSynthesis: buildFragmentPreviews
FinalSynthesis->>Mapper: mapFragmentsToSections(fragments,<br/>plan)
Mapper-->>FinalSynthesis: fragment-to-section<br/>assignments
FinalSynthesis->>FinalSynthesis: selectImagesForFragmentIds<br/>and selectMappedEntries
loop For each section
FinalSynthesis->>Model: synthesizeSection(context,<br/>section payload)
Model-->>FinalSynthesis: section answer
end
else Single Mode
FinalSynthesis->>FinalSynthesis: selectImagesForFinalSynthesis
FinalSynthesis->>Model: synthesizeSingleAnswer(context,<br/>full payload)
Model-->>FinalSynthesis: answer
end
FinalSynthesis->>FinalSynthesis: Stream answer chunks
FinalSynthesis-->>MessageAgent: FinalSynthesisExecutionResult
MessageAgent-->>Client: Final answer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the AI agent's ability to generate final answers, particularly when dealing with extensive evidence. By introducing a flexible synthesis pipeline that can dynamically switch between single and sectional processing modes, the system can now efficiently handle large context windows, improve answer quality, and reduce the risk of context length errors. The changes also centralize and streamline the underlying logic, making the system more scalable and easier to maintain. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 introduces a new, sophisticated final answer synthesis mechanism, primarily implemented in a new file server/api/chat/final-answer-synthesis.ts. This module intelligently decides between a single-shot or a multi-sectional approach for generating answers, especially when dealing with large amounts of context and images, to stay within model token limits. It includes logic for planning sections, mapping fragments to these sections, and synthesizing each section concurrently. To support this, server/ai/modelConfig.ts was updated to store and retrieve maximum input token limits for various models, and server/shared/types.ts was modified to include maxInputTokens in the ModelConfiguration interface. The existing server/api/chat/message-agents.ts file was refactored to delegate the entire final synthesis process to the new module, simplifying its code. Additionally, a new test file server/tests/finalAnswerSynthesis.test.ts was added to validate the new synthesis logic, covering fragment preview generation, sectional payload construction, mode selection, and fragment budget management. A minor indentation issue was noted in the selectMappedEntriesWithinBudget function call within the new synthesis file.
Note: Security Review did not run due to the size of the PR.
| selectMappedEntriesWithinBudget( | ||
| orderedEntries, | ||
| baseTokens, | ||
| safeInputBudget, | ||
| ) |
There was a problem hiding this comment.
There's a minor indentation issue here that affects readability. The arguments to selectMappedEntriesWithinBudget are indented incorrectly.
| selectMappedEntriesWithinBudget( | |
| orderedEntries, | |
| baseTokens, | |
| safeInputBudget, | |
| ) | |
| selectMappedEntriesWithinBudget( | |
| orderedEntries, | |
| baseTokens, | |
| safeInputBudget, | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/chat/message-agents.ts (1)
3092-3118:⚠️ Potential issue | 🟠 MajorReset the full final-synthesis state on every non-success exit.
review.lockedByFinalSynthesisis set before the streaming-channel guard and never cleared on the early return or either catch path. After a recoverable synthesis failure, the run will skip all later reviews/replanning, and the leftoverstreamedText/ackReceivedcan leak partial output into a retry or delegated fallback.Proposed fix
async execute(_args, context) { const mutableContext = mutableAgentContext(context) - if (!mutableContext.review.lockedByFinalSynthesis) { - mutableContext.review.lockedByFinalSynthesis = true - mutableContext.review.lockedAtTurn = - mutableContext.turnCount ?? MIN_TURN_NUMBER - loggerWithChild({ email: context.user.email }).info( - { - chatId: context.chat.externalId, - turn: mutableContext.review.lockedAtTurn, - }, - "[MessageAgents][FinalSynthesis] Review lock activated after synthesis tool call." - ) - } + const resetFinalSynthesisState = () => { + mutableContext.finalSynthesis.requested = false + mutableContext.finalSynthesis.completed = false + mutableContext.finalSynthesis.suppressAssistantStreaming = false + mutableContext.finalSynthesis.streamedText = "" + mutableContext.finalSynthesis.ackReceived = false + mutableContext.review.lockedByFinalSynthesis = false + mutableContext.review.lockedAtTurn = null + } + if ( mutableContext.finalSynthesis.requested && mutableContext.finalSynthesis.completed ) { return ToolResponse.error( @@ if (!mutableContext.runtime?.streamAnswerText) { return ToolResponse.error( "EXECUTION_FAILED", "Streaming channel unavailable. Cannot deliver final answer." ) } + + if (!mutableContext.review.lockedByFinalSynthesis) { + mutableContext.review.lockedByFinalSynthesis = true + mutableContext.review.lockedAtTurn = + mutableContext.turnCount ?? MIN_TURN_NUMBER + loggerWithChild({ email: context.user.email }).info( + { + chatId: context.chat.externalId, + turn: mutableContext.review.lockedAtTurn, + }, + "[MessageAgents][FinalSynthesis] Review lock activated after synthesis tool call." + ) + } @@ } catch (error) { + resetFinalSynthesisState() if (isMessageAgentStopError(error)) { - context.finalSynthesis.suppressAssistantStreaming = false - context.finalSynthesis.requested = false - context.finalSynthesis.completed = false throw error } - - context.finalSynthesis.suppressAssistantStreaming = false - context.finalSynthesis.requested = false - context.finalSynthesis.completed = false loggerWithChild({ email: context.user.email }).error( { err: error instanceof Error ? error.message : String(error) }, "Final synthesis tool failed." )Also applies to: 3160-3179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/chat/message-agents.ts` around lines 3092 - 3118, The final-synthesis state (e.g., mutableContext.review.lockedByFinalSynthesis, mutableContext.review.lockedAtTurn and all fields under mutableContext.finalSynthesis such as streamedText and ackReceived) is set before the streaming-channel guard but not cleared on early returns or failure paths; update the control flow so that any non-success exit (the early return when runtime?.streamAnswerText is false, the "Final synthesis already completed" path, and all catch/failure branches around the synthesis call) resets the entire final-synthesis state to its initial/empty values and clears the review lock (unset lockedByFinalSynthesis and lockedAtTurn) to avoid leaking partial output into retries or later stages, ensuring you touch the same symbols (mutableContext.review.lockedByFinalSynthesis, mutableContext.review.lockedAtTurn, mutableContext.finalSynthesis.streamedText, mutableContext.finalSynthesis.ackReceived, and mutableContext.finalSynthesis.requested/completed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@server/api/chat/message-agents.ts`:
- Around line 3092-3118: The final-synthesis state (e.g.,
mutableContext.review.lockedByFinalSynthesis, mutableContext.review.lockedAtTurn
and all fields under mutableContext.finalSynthesis such as streamedText and
ackReceived) is set before the streaming-channel guard but not cleared on early
returns or failure paths; update the control flow so that any non-success exit
(the early return when runtime?.streamAnswerText is false, the "Final synthesis
already completed" path, and all catch/failure branches around the synthesis
call) resets the entire final-synthesis state to its initial/empty values and
clears the review lock (unset lockedByFinalSynthesis and lockedAtTurn) to avoid
leaking partial output into retries or later stages, ensuring you touch the same
symbols (mutableContext.review.lockedByFinalSynthesis,
mutableContext.review.lockedAtTurn, mutableContext.finalSynthesis.streamedText,
mutableContext.finalSynthesis.ackReceived, and
mutableContext.finalSynthesis.requested/completed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0bc99031-2b58-42ec-bc34-f430af9bf1ad
📒 Files selected for processing (6)
server/ai/modelConfig.tsserver/api/chat/final-answer-synthesis.tsserver/api/chat/message-agents.tsserver/logger/index.tsserver/shared/types.tsserver/tests/finalAnswerSynthesis.test.ts
2 participants
Description
Testing
Additional Notes
Summary by CodeRabbit
Release Notes
New Features
Chores