feat: link fix suggestions to summary via [S:id] markers#269
Merged
Conversation
Previously, fix suggestions created by github_fix during RCA were matched to summary "Suggested Next Steps" items using word overlap heuristics. This frequently failed when the summary wording diverged from the suggestion title, leaving the fix button invisible to users. Backend: the summarizer now fetches pre-existing type='fix' suggestions and includes them in the LLM prompt with [S:id] markers. The LLM emits these markers inline next to the relevant next step. Frontend: the citation parser now recognizes [S:id] markers and renders them as green GitBranch buttons that open the FixSuggestionModal directly. Word-matching is skipped for list items that already have a marker, preventing duplicates. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 20 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR adds end-to-end support for inline fix-suggestion markers ( ChangesFix-suggestion markers end-to-end
Sequence DiagramssequenceDiagram
participant IncidentCard
participant TextRenderer
participant FixAction
IncidentCard->>TextRenderer: renderTextWithCitations(text)
TextRenderer->>TextRenderer: split on [S:id] markers
alt marker found
TextRenderer->>TextRenderer: findFixSuggestionById(id)
alt canWrite enabled
TextRenderer->>FixAction: render GitBranch button
FixAction->>IncidentCard: setSelectedFixSuggestion(id)
else canWrite disabled
TextRenderer-->>TextRenderer: render null
end
else citation found
TextRenderer->>FixAction: render citation link
end
sequenceDiagram
participant SummarizationTask
participant FetchHelper
participant Database
SummarizationTask->>FetchHelper: _fetch_fix_suggestions(incident_id)
FetchHelper->>Database: query incident_suggestions where type='fix'
alt success
Database-->>FetchHelper: rows
FetchHelper-->>SummarizationTask: [{ id, title, description, file_path, repository }, ...]
else transient error with retries
FetchHelper->>Database: retry with backoff
else exhausted retries
FetchHelper-->>SummarizationTask: []
end
SummarizationTask->>SummarizationTask: log count if non-empty
sequenceDiagram
participant SummarizationService
participant PromptBuilder
participant LLM
SummarizationService->>PromptBuilder: _build_summary_prompt_with_chat(fix_suggestions=[...])
alt fix_suggestions provided
PromptBuilder->>PromptBuilder: build CODE FIX SUGGESTIONS block with [S:id] markers
PromptBuilder->>PromptBuilder: append to prompt with usage instructions
end
PromptBuilder->>LLM: send complete prompt (both citation and transcript paths)
LLM-->>SummarizationService: incident summary (may include [S:id] refs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/incidents/components/IncidentCard.tsx`:
- Around line 186-207: The code currently only handles suggestion markers when
canWrite is true, leaving raw "[S:id]" tokens visible to read-only users; modify
the logic in the suggestion-handling block (the suggestionMatch check that uses
findFixSuggestionById and setSelectedFixSuggestion) to first check for
suggestionMatch unconditionally and, if present, always consume the marker by
returning null for non-writers, and only render the GitBranch button when
canWrite and a suggestion exists; in other words: change the condition from if
(suggestionMatch && canWrite) { ... } to if (suggestionMatch) { const suggestion
= findFixSuggestionById(...); if (canWrite && suggestion) return (<button ...
/>); return null; }.
In `@server/chat/background/summarization.py`:
- Around line 338-365: The _fetch_fix_suggestions() block currently catches all
exceptions, logs a minimal warning, and returns [], hiding transient DB errors;
instead log the full exception (use logger.exception or include traceback) and
re-raise the exception so the outer generate_incident_summary_from_chat() /
Celery retry path can handle it; specifically update the try/except around
db_pool.get_admin_connection() / cursor.execute() to replace logger.warning(...)
with a full exception log and then raise the caught exception (or omit
swallowing) rather than returning an empty list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 935383a3-bfb0-4deb-8f8f-ed4008117433
📒 Files selected for processing (2)
client/src/app/incidents/components/IncidentCard.tsxserver/chat/background/summarization.py
…gestion fetch - IncidentCard: always match [S:id] markers so read-only users don't see raw tokens - summarization: retry _fetch_fix_suggestions up to 2 times on transient DB failures Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gestion-markers # Conflicts: # server/chat/background/summarization.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/chat/backend/agent/tools/mcp_tools.py (1)
307-311: 🧹 Nitpick | 🔵 TrivialStale comment — token is no longer passed via Docker
-e.This branch is now reachable for
server_type == "github"but the comment claims the token was set via Docker args; actually the new GitHub-specific env block at Lines 249–258 is what writesGITHUB_PERSONAL_ACCESS_TOKEN. Thepassis functionally fine, but the comment will mislead the next reader.♻️ Update the stale Docker comment
- # GitHub credentials are passed via Docker -e flag in the command itself - # (handled above when building the docker command) elif server_type == "github": - # Token already passed in Docker command args, nothing to add to env + # GITHUB_PERSONAL_ACCESS_TOKEN / GITHUB_TOOLSETS are already + # set in the GitHub-specific env block above; nothing to add here. pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/chat/backend/agent/tools/mcp_tools.py` around lines 307 - 311, Update the stale inline comment in the server_type == "github" branch in mcp_tools.py: replace the misleading text that says "Token already passed in Docker command args" with a concise note that the GitHub token is set by the GitHub-specific env block (writes GITHUB_PERSONAL_ACCESS_TOKEN), or remove the comment entirely; keep the pass statement as-is (the branch is intentionally no-op) and reference the GitHub env block handling around the earlier GITHUB_PERSONAL_ACCESS_TOKEN assignment to clarify where the token is actually injected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/app/incidents/components/IncidentCard.tsx`:
- Around line 296-298: The code currently sets hasSuggestionMarker and, if true,
suppresses findMatchingSuggestion for the whole bullet which hides non-fix
actions whenever any [S:id] marker appears; instead remove the markers from the
input passed to the matcher so findMatchingSuggestion can still detect other
actions. Concretely, keep findMatchingSuggestion but call it with a cleaned
string (e.g., const cleanedText = textContent.replace(/\[S:\d+\]/g, '').trim())
and compute matchingSuggestion from cleanedText; you can keep
hasSuggestionMarker for any other logic but do not use it to short-circuit
calling findMatchingSuggestion. Ensure you update the matchingSuggestion
assignment (and any downstream code that depends on matchingSuggestion) to use
the cleanedText result.
In `@server/chat/backend/agent/tools/github_mcp_utils.py`:
- Around line 141-146: The current fallback in parse_file_content decodes base64
raw bytes with latin-1 which will silently accept binary blobs; change the logic
in github_mcp_utils.py (the base64.b64decode -> raw decoding block where
variables raw and decoded are used) to first detect obvious binary (e.g., if
b'\x00' in raw or a high non-text byte ratio) and treat those as
non-text/binary, otherwise decode using utf-8 with a safe fallback such as
raw.decode("utf-8", errors="replace") (or if you keep a fallback branch, avoid
using plain latin-1 and instead mark replacements) so binary artifacts are not
passed through as garbled source for the suggestion pipeline.
In `@server/chat/backend/agent/tools/mcp_tools.py`:
- Line 237: Remove the unnecessary f-string prefix from the logging call in
mcp_tools.py (the logging.info line that currently reads f" GitHub MCP server
command prepared (npx with all toolsets)"); change it to a plain string and also
adjust the message to not claim "all toolsets" since GITHUB_TOOLSETS is not used
by the archived `@modelcontextprotocol/server-github` package—e.g., use a neutral
phrase like "with npx and configured toolsets" to avoid misleading wording while
keeping the log location tied to the existing logging.info call.
- Around line 120-121: The code currently invokes the deprecated npm package
"@modelcontextprotocol/server-github" via npx in the MCP server command (see the
command entry using npx) and force-restarts it on every RCA call (the restart
logic referenced as the RCA restart), which causes security, performance, and
toolset-recognition issues (GITHUB_TOOLSETS and allowed_github_mcp_tools
filtering). Replace the npx invocation with the official GitHub MCP server
deployment (use the ghcr.io/github/github-mcp-server Docker image or build/run
the Go binary from github.com/github/github-mcp-server), preserve and use the
existing environment-based token injection (keep the env-var token logic)
instead of Docker-arg comments, remove the per-RCA forced restart so the server
is reused between calls, and update the stale comment about Docker token passage
to reflect current env-based token injection and ensure GITHUB_TOOLSETS is
passed/recognized by the chosen server implementation.
---
Outside diff comments:
In `@server/chat/backend/agent/tools/mcp_tools.py`:
- Around line 307-311: Update the stale inline comment in the server_type ==
"github" branch in mcp_tools.py: replace the misleading text that says "Token
already passed in Docker command args" with a concise note that the GitHub token
is set by the GitHub-specific env block (writes GITHUB_PERSONAL_ACCESS_TOKEN),
or remove the comment entirely; keep the pass statement as-is (the branch is
intentionally no-op) and reference the GitHub env block handling around the
earlier GITHUB_PERSONAL_ACCESS_TOKEN assignment to clarify where the token is
actually injected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ef69909-62c9-42bc-b525-8253bda39f6c
📒 Files selected for processing (3)
client/src/app/incidents/components/IncidentCard.tsxserver/chat/backend/agent/tools/github_mcp_utils.pyserver/chat/backend/agent/tools/mcp_tools.py
|
Resolve conflicts: - mcp_tools.py: keep native github-mcp-server binary (from main) - summarization.py: combine fix_suggestions (branch) with agent_reasoning (main)
damianloch
requested changes
Jun 2, 2026
- IncidentCard: viewers can see suggestion markers (disabled, not hidden) - Revert github_mcp_utils.py (fixed upstream) - Revert validate-env-vars.sh (not needed) - Revert mcp_tools.py (fixed upstream) - Remove dead code in summarization.py line 384 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
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)
client/src/app/incidents/components/IncidentCard.tsx (1)
338-390: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueInconsistent permission handling: marker buttons vs. word-matching buttons.
Marker-based GitBranch buttons (lines 238–245) are visible-but-disabled for viewers, while word-matching GitBranch and Play buttons (lines 345–390) are completely hidden when
!canWrite. This creates two different UX patterns:
[S:id]marker buttons: viewers see a disabled button (can't interact)- Word-matched suggestions: viewers see nothing
If this distinction is intentional (e.g., marker buttons are high-fidelity LLM pointers that should be visible to indicate "a fix was suggested here," while heuristic matches are less reliable and shouldn't clutter the viewer's UI), consider adding a code comment to document the rationale. Otherwise, unify the pattern—either hide both or show both as disabled for viewers.
🤖 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 `@client/src/app/incidents/components/IncidentCard.tsx` around lines 338 - 390, The code shows marker-based buttons for viewers as visible-but-disabled but hides word-matching suggestion buttons entirely when !canWrite; update the behavior to be consistent by either (A) always render the suggestion button when matchingSuggestion exists but mark it disabled/uninteractive for viewers, or (B) explicitly document the intent with a comment. To implement (A) change the canShowAction logic (used to decide rendering) so it doesn't gate visibility on canWrite alone (e.g., allow rendering when matchingSuggestion exists) and instead apply disabled behavior and UI tweaks when !canWrite: add the disabled attribute and a non-interactive CSS state (e.g., cursor-not-allowed/opacity) to the button render path that uses isFixType, setSelectedFixSuggestion/setSelectedSuggestion, matchingSuggestion, canShowAction and wasExecuted/execStatus so viewers still see the indicator but cannot interact; alternatively, if choosing (B) add a clear comment near canShowAction explaining why marker buttons are visible for viewers but heuristic matches are hidden.
🤖 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.
Outside diff comments:
In `@client/src/app/incidents/components/IncidentCard.tsx`:
- Around line 338-390: The code shows marker-based buttons for viewers as
visible-but-disabled but hides word-matching suggestion buttons entirely when
!canWrite; update the behavior to be consistent by either (A) always render the
suggestion button when matchingSuggestion exists but mark it
disabled/uninteractive for viewers, or (B) explicitly document the intent with a
comment. To implement (A) change the canShowAction logic (used to decide
rendering) so it doesn't gate visibility on canWrite alone (e.g., allow
rendering when matchingSuggestion exists) and instead apply disabled behavior
and UI tweaks when !canWrite: add the disabled attribute and a non-interactive
CSS state (e.g., cursor-not-allowed/opacity) to the button render path that uses
isFixType, setSelectedFixSuggestion/setSelectedSuggestion, matchingSuggestion,
canShowAction and wasExecuted/execStatus so viewers still see the indicator but
cannot interact; alternatively, if choosing (B) add a clear comment near
canShowAction explaining why marker buttons are visible for viewers but
heuristic matches are hidden.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26756e83-c598-4ff3-b2c8-c16c634a1496
📒 Files selected for processing (2)
client/src/app/incidents/components/IncidentCard.tsxserver/chat/background/summarization.py
💤 Files with no reviewable changes (1)
- server/chat/background/summarization.py
…mplicit (fall through) returns' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
|
damianloch
approved these changes
Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
github_fixduring RCA were invisible in the UI because the word-matching heuristic between summary text and suggestion titles often failed when wording divergedtype='fix'suggestions into the LLM prompt, which emits[S:id]markers inline next to the relevant "Suggested Next Steps" item[S:id]markers and renders them as green GitBranch buttons that open the FixSuggestionModal directly — no word-matching neededHow it works
Backend (
server/chat/background/summarization.py):_fetch_fix_suggestions()queriesincident_suggestions WHERE type = 'fix'before summary generationCODE FIX SUGGESTIONSblock with[S:id]markersFrontend (
client/src/app/incidents/components/IncidentCard.tsx):renderTextWithCitationsregex extended to also split on[S:\d+]patterns[S:id]markers render as clickable GitBranch buttons that openFixSuggestionModal<li>handler skips word-matching fallback when a[S:id]marker is present (prevents duplicate buttons)Test plan
github_fixsuggestion[S:id]marker in the next steps[S:]markers, word-matching still works)🤖 Generated with Claude Code
Summary by CodeRabbit