fix: make citation labels more precise and observable#140
Closed
yilu331 wants to merge 1 commit into
Closed
Conversation
Stop pre-building multi-item "copy this marker" instructions so models are not nudged to over-cite every injected memory. Resolve unique rank fingerprint drift so true markers still count, persist unresolved marker ids locally for diagnostics (not on the publish wire), and show injected vs applied counts in the dashboard.
📝 WalkthroughWalkthroughThe change refines citation marker instructions and resolution, records unresolved citation identifiers, counts unique injected entries per session, and exposes both injected counts and unresolved markers in the dashboard. ChangesCitation and dashboard tracking
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AssistantText
participant StopHook
participant SessionRecord
participant DashboardReader
participant DashboardUI
AssistantText->>StopHook: provide citation markers
StopHook->>SessionRecord: publish cited items and unresolved citations
DashboardReader->>SessionRecord: read session and injected records
DashboardReader-->>DashboardUI: return turn markers and injected count
DashboardUI-->>DashboardUI: render badges and unresolved marker rows
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@plugin/src/claude_smart/events/stop.py`:
- Around line 299-306: Update the citation-processing loop around
_registry_entry_for_citation to deduplicate resolved entries using the canonical
entry["id"], while maintaining a separate set for unresolved citation tokens.
Ensure case variants or different tokens resolving to the same registry entry
produce only one applied item, without changing unresolved-token handling.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64da18a8-e432-4d38-a244-0936955998f7
📒 Files selected for processing (11)
plugin/dashboard/app/dashboard/page.tsxplugin/dashboard/app/sessions/[sessionId]/page.tsxplugin/dashboard/app/sessions/page.tsxplugin/dashboard/components/common/injected-badge.tsxplugin/dashboard/lib/session-reader.tsplugin/dashboard/lib/types.tsplugin/src/claude_smart/context_format.pyplugin/src/claude_smart/events/stop.pyplugin/src/claude_smart/state.pytests/test_context_format.pytests/test_events.py
Comment on lines
299
to
306
| seen: set[str] = set() | ||
| resolved: list[dict[str, Any]] = [] | ||
| unresolved: list[str] = [] | ||
| for cid in cited_ids: | ||
| if cid in seen: | ||
| continue | ||
| seen.add(cid) | ||
| entry = _registry_entry_for_citation(registry, cid) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Deduplicate by the resolved registry ID.
Case variants or multiple drifted tokens can resolve to the same entry but bypass seen, producing duplicate applied items. Track the canonical entry["id"] after resolution, while separately deduplicating unresolved tokens.
Proposed fix
- seen: set[str] = set()
+ seen_resolved: set[str] = set()
+ seen_unresolved: set[str] = set()
resolved: list[dict[str, Any]] = []
unresolved: list[str] = []
for cid in cited_ids:
- if cid in seen:
- continue
- seen.add(cid)
entry = _registry_entry_for_citation(registry, cid)
if not entry:
+ unresolved_key = cid.lower()
+ if unresolved_key in seen_unresolved:
+ continue
+ seen_unresolved.add(unresolved_key)
unresolved.append(cid)
continue
+ canonical_id = str(entry.get("id") or cid).lower()
+ if canonical_id in seen_resolved:
+ continue
+ seen_resolved.add(canonical_id)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| seen: set[str] = set() | |
| resolved: list[dict[str, Any]] = [] | |
| unresolved: list[str] = [] | |
| for cid in cited_ids: | |
| if cid in seen: | |
| continue | |
| seen.add(cid) | |
| entry = _registry_entry_for_citation(registry, cid) | |
| seen_resolved: set[str] = set() | |
| seen_unresolved: set[str] = set() | |
| resolved: list[dict[str, Any]] = [] | |
| unresolved: list[str] = [] | |
| for cid in cited_ids: | |
| entry = _registry_entry_for_citation(registry, cid) | |
| if not entry: | |
| unresolved_key = cid.lower() | |
| if unresolved_key in seen_unresolved: | |
| continue | |
| seen_unresolved.add(unresolved_key) | |
| unresolved.append(cid) | |
| continue | |
| canonical_id = str(entry.get("id") or cid).lower() | |
| if canonical_id in seen_resolved: | |
| continue | |
| seen_resolved.add(canonical_id) |
🤖 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 `@plugin/src/claude_smart/events/stop.py` around lines 299 - 306, Update the
citation-processing loop around _registry_entry_for_citation to deduplicate
resolved entries using the canonical entry["id"], while maintaining a separate
set for unresolved citation tokens. Ensure case variants or different tokens
resolving to the same registry entry produce only one applied item, without
changing unresolved-token handling.
Contributor
|
Superseded by PR from wenchanghan fork. |
Collaborator
Author
|
Superseded by #141 (wenchanghan fork). |
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
2 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
Context
FP (over-cite) is worse than FN. These changes improve label precision and observability without loosening the material-change cite bar or inventing applied credit.
Test plan
pytest tests/test_context_format.py tests/test_events.py tests/test_cs_cite.py tests/test_state.py tests/test_codex_support.pytsc --noEmit/sessionsand confirm injected badge appears on sessions with registry files; open a turn with a bad marker id and confirm unresolved row (after install from this branch)Summary by CodeRabbit
New Features
Improvements