Only credit the learnings the assistant says it used#141
Merged
yyiilluu merged 1 commit intoJul 21, 2026
Conversation
3 tasks
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR changes citation terminology and metrics across the citation pipeline and dashboard, preserves unresolved citation IDs, adds injected-learning counts, and updates session, dashboard, skills, and preferences views to display cited-learning statistics. ChangesCitation pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Assistant
participant StopHook
participant SessionReader
participant Dashboard
Assistant->>StopHook: emit citation marker
StopHook->>SessionReader: store cited_items and unresolved_citations
SessionReader->>Dashboard: provide citation and injection metrics
Dashboard-->>Dashboard: render cited and unresolved-citation status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/src/claude_smart/events/stop.py (1)
299-321: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeduplicate the resolved registry entry, not only the raw citation token.
Different aliases such as an exact ID and a stale fingerprint can resolve to the same entry, producing duplicate
cited_itemsand inflated applied counts. Normalize raw tokens and track the resolved entry’s canonical ID.Proposed fix
- seen: set[str] = set() + seen_citations: set[str] = set() + seen_entries: set[str] = set() resolved: list[dict[str, Any]] = [] unresolved: list[str] = [] for cid in cited_ids: - if cid in seen: + normalized_cid = cid.casefold() + if normalized_cid in seen_citations: continue - seen.add(cid) + seen_citations.add(normalized_cid) entry = _registry_entry_for_citation(registry, cid) if not entry: unresolved.append(cid) continue + canonical_id = str(entry.get("id") or cid).casefold() + if canonical_id in seen_entries: + continue + seen_entries.add(canonical_id) item: dict[str, Any] = {🤖 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 - 321, Update the citation loop in the stop-event resolution logic to normalize each raw token before lookup and deduplicate using the resolved registry entry’s canonical ID, rather than only the raw citation token. Ensure aliases such as an exact ID and stale fingerprint produce one resolved item and do not inflate applied counts, while preserving unresolved-token handling.
🧹 Nitpick comments (1)
plugin/dashboard/lib/session-reader.ts (1)
438-449: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize I/O for reading session and injected data.
The addition of
await countUniqueInjected(sessionId)inside thisforloop introduces a second sequential file read per session. To improve dashboard load times without risking file-descriptor limits (which could happen if we parallelized all sessions at once), you can read the two files for each session concurrently.⚡ Proposed refactor
- const records = await readJsonl(fullPath).catch(() => []); + const [records, injectedLearningCount] = await Promise.all([ + readJsonl(fullPath).catch(() => []), + countUniqueInjected(sessionId), + ]); const { turns, publishedUpTo, learningInteractionCount, lastTs, firstTs, preview, host, requestIds, } = foldTurns(records); - const injectedLearningCount = await countUniqueInjected(sessionId);🤖 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/dashboard/lib/session-reader.ts` around lines 438 - 449, Within the session-processing loop, start the session JSONL read and countUniqueInjected(sessionId) concurrently, then await both results together before calling foldTurns. Preserve the existing fallback of [] for readJsonl failures and keep concurrency scoped to each session rather than parallelizing across sessions.
🤖 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/context_format.py`:
- Around line 353-357: Update the available-memory label in the context
formatting flow around marker_parts so it does not call every linked entry
“Qualifying.” Use neutral wording that reflects these are linked titles pending
the model’s relevance decision, while preserving the existing joined list and
punctuation.
---
Outside diff comments:
In `@plugin/src/claude_smart/events/stop.py`:
- Around line 299-321: Update the citation loop in the stop-event resolution
logic to normalize each raw token before lookup and deduplicate using the
resolved registry entry’s canonical ID, rather than only the raw citation token.
Ensure aliases such as an exact ID and stale fingerprint produce one resolved
item and do not inflate applied counts, while preserving unresolved-token
handling.
---
Nitpick comments:
In `@plugin/dashboard/lib/session-reader.ts`:
- Around line 438-449: Within the session-processing loop, start the session
JSONL read and countUniqueInjected(sessionId) concurrently, then await both
results together before calling foldTurns. Preserve the existing fallback of []
for readJsonl failures and keep concurrency scoped to each session rather than
parallelizing across sessions.
🪄 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: 96617689-7578-47ae-9a25-835615163504
📒 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
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)
plugin/dashboard/lib/session-reader.ts (1)
450-485: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winParallelize session file reads.
Currently,
listSessions()iterates over the directory entries sequentially, waiting for each session's file reads to complete before starting the next. As noted in the PR objectives, mapping the entries to promises and awaiting them concurrently viaPromise.all()will significantly improve the dashboard's load performance.⚡ Proposed fix
- const summaries: SessionSummary[] = []; - for (const entry of entries) { - if (!entry.endsWith(".jsonl")) continue; - if (entry.endsWith(".injected.jsonl")) continue; - const fullPath = path.join(dir, entry); - const sessionId = entry.replace(/\.jsonl$/, ""); - const [records, injectedLearningCount] = await Promise.all([ - readJsonl(fullPath).catch(() => []), - countUniqueInjected(sessionId), - ]); - const { - turns, - publishedUpTo, - learningInteractionCount, - appliedLearningCount, - lastTs, - firstTs, - preview, - host, - requestIds, - } = foldTurns(records); - summaries.push({ - session_id: sessionId, - turn_count: turns.length, - learning_interaction_count: learningInteractionCount, - applied_learning_count: appliedLearningCount, - injected_learning_count: injectedLearningCount, - last_activity: lastTs, - first_activity: firstTs, - published_up_to: publishedUpTo, - preview, - source: "local", - host, - request_ids: requestIds, - }); - } + const sessionPromises = entries + .filter((entry) => entry.endsWith(".jsonl") && !entry.endsWith(".injected.jsonl")) + .map(async (entry): Promise<SessionSummary> => { + const fullPath = path.join(dir, entry); + const sessionId = entry.replace(/\.jsonl$/, ""); + const [records, injectedLearningCount] = await Promise.all([ + readJsonl(fullPath).catch(() => []), + countUniqueInjected(sessionId), + ]); + const { + turns, + publishedUpTo, + learningInteractionCount, + appliedLearningCount, + lastTs, + firstTs, + preview, + host, + requestIds, + } = foldTurns(records); + return { + session_id: sessionId, + turn_count: turns.length, + learning_interaction_count: learningInteractionCount, + applied_learning_count: appliedLearningCount, + injected_learning_count: injectedLearningCount, + last_activity: lastTs, + first_activity: firstTs, + published_up_to: publishedUpTo, + preview, + source: "local", + host, + request_ids: requestIds, + }; + }); + + const summaries = await Promise.all(sessionPromises);🤖 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/dashboard/lib/session-reader.ts` around lines 450 - 485, Update listSessions() to map eligible session entries to promises and await them with Promise.all(), so readJsonl() and countUniqueInjected() for all sessions run concurrently. Preserve the existing filtering, foldTurns() processing, and SessionSummary fields, excluding invalid or injected entries as before.
🤖 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 `@plugin/dashboard/lib/session-reader.ts`:
- Around line 450-485: Update listSessions() to map eligible session entries to
promises and await them with Promise.all(), so readJsonl() and
countUniqueInjected() for all sessions run concurrently. Preserve the existing
filtering, foldTurns() processing, and SessionSummary fields, excluding invalid
or injected entries as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7cc57564-3bb0-4b10-9e71-a9c75635a292
📒 Files selected for processing (10)
plugin/dashboard/app/dashboard/page.tsxplugin/dashboard/app/sessions/[sessionId]/page.tsxplugin/dashboard/app/sessions/page.tsxplugin/dashboard/components/common/learnings-badge.tsxplugin/dashboard/lib/session-reader.tsplugin/dashboard/lib/types.tsplugin/src/claude_smart/context_format.pyplugin/src/claude_smart/events/stop.pytests/test_context_format.pytests/test_events.py
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin/dashboard/app/sessions/page.tsx
- plugin/dashboard/app/sessions/[sessionId]/page.tsx
- plugin/dashboard/app/dashboard/page.tsx
- plugin/dashboard/lib/types.ts
- plugin/src/claude_smart/context_format.py
- tests/test_context_format.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/dashboard/lib/session-reader.ts (1)
450-485: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRead sessions concurrently to avoid N+1 file I/O latency.
Both
listSessionsandlistCitedRulesiterate over the session entries sequentially, awaiting each file read before moving to the next. For larger numbers of sessions, this sequential processing significantly increases dashboard load times. You can usePromise.allto read all valid entries concurrently before aggregating them.
plugin/dashboard/lib/session-reader.ts#L450-L485: Replace theforloop inlistSessionswithawait Promise.all(entries.filter(...).map(async (entry) => ...)).plugin/dashboard/lib/session-reader.ts#L242-L293: Apply the same parallelization pattern inlistCitedRulesto concurrently fetch therecordsarrays, then flat-map or reduce them into thestatsmap.🤖 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/dashboard/lib/session-reader.ts` around lines 450 - 485, Parallelize session processing in listSessions by replacing its sequential loop with Promise.all over valid entries, preserving each entry’s filtering, record reading, counting, folding, and summary construction. Apply the same concurrent record-fetch pattern in listCitedRules, then aggregate the fetched results into the existing stats map. Affected sites: plugin/dashboard/lib/session-reader.ts lines 450-485 (update listSessions); plugin/dashboard/lib/session-reader.ts lines 242-293 (update listCitedRules).
🤖 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 `@plugin/dashboard/lib/session-reader.ts`:
- Around line 450-485: Parallelize session processing in listSessions by
replacing its sequential loop with Promise.all over valid entries, preserving
each entry’s filtering, record reading, counting, folding, and summary
construction. Apply the same concurrent record-fetch pattern in listCitedRules,
then aggregate the fetched results into the existing stats map. Affected sites:
plugin/dashboard/lib/session-reader.ts lines 450-485 (update listSessions);
plugin/dashboard/lib/session-reader.ts lines 242-293 (update listCitedRules).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c110e82-b6c8-4fb1-84a1-3c3f96111112
📒 Files selected for processing (18)
README.mdplugin/dashboard/app/api/rules/cited/route.tsplugin/dashboard/app/dashboard/page.tsxplugin/dashboard/app/preferences/page.tsxplugin/dashboard/app/sessions/[sessionId]/page.tsxplugin/dashboard/app/sessions/page.tsxplugin/dashboard/app/skills/page.tsxplugin/dashboard/components/common/cited-learnings-badge.tsxplugin/dashboard/components/common/injected-badge.tsxplugin/dashboard/components/common/learning-citation-badge.tsxplugin/dashboard/lib/session-reader.tsplugin/dashboard/lib/types.tsplugin/src/claude_smart/cs_cite.pytests/host_learning_harness.pytests/test_codex_support.pytests/test_context_format.pytests/test_cs_cite.pytests/test_events.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_context_format.py
wenchanghan
changed the title
wenchanghan
changed the title
wenchanghan
changed the title
wenchanghan
changed the title
wenchanghan
force-pushed
the
codex/citation-labeling-precision
branch
from
July 21, 2026 07:51
451deae to
1dd3ad9
Compare
wenchanghan
changed the title
wenchanghan
force-pushed
the
codex/citation-labeling-precision
branch
from
July 21, 2026 08:14
1dd3ad9 to
313054e
Compare
wenchanghan
changed the title
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.
Customer impact
When several learnings are available, claude-smart no longer gives the assistant an Applied footer prefilled with all of them. The assistant now sees a one-learning format example and is told to include only the learnings that materially changed its response.
This removes prompt pressure to credit unrelated learnings while keeping the existing Learning applied wording, clickable links, and dashboard behavior.
Before
If learnings A, B, and C were retrieved and only A helped, the prompt still told the assistant to copy a footer containing A, B, and C.
After
The prompt shows A as the format example, keeps B and C available to link, and tells the assistant to add only the learnings that materially changed the response. Legitimate multi-learning attribution remains supported.
Scope
Validation
Supersedes #140.