fix(app): resolve production citation and guided-learning regressions - #111
Conversation
📝 WalkthroughWalkthroughFrontend routing, chat focus behavior, citation identity handling, responsive lesson layout, and backend official-documentation citation retrieval were updated. New tests cover route recovery, streaming focus, citation anchors, sparse search, deadlines, failures, and future cancellation. ChangesFrontend interaction and citation updates
Official documentation citation retrieval
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes production regressions around citation discovery (incorrect sources, expensive retrieval paths, timeout budgeting, and anchor handling) and guided-learning UX (route recovery, input focus restoration, and mobile reading clearance). It tightens citation retrieval to a single sparse, official-documentation-only Qdrant request, and standardizes citation identity handling across backend and frontend.
Changes:
- Backend: route citation discovery through a sparse-only Qdrant query with a shared timeout deadline and cancellation propagation, returning structured citation outcomes with base URL + anchor split.
- Backend: centralize canonical “official documentation source identities” projection to avoid restating manifest-owned source lists.
- Frontend: deduplicate citations by case-insensitive base URL + case-sensitive anchor, recover unsupported nested learn routes to
/learn, and restore chat input focus after streaming without stealing external focus (plus mobile layout clearance).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/williamcallahan/javachat/service/RetrievalServiceTest.java | Updates retrieval tests for sparse-only citation discovery, dedupe/limit behavior, and strict failure propagation. |
| src/test/java/com/williamcallahan/javachat/service/RetrievalServiceCitationTest.java | Adds focused tests for citation URL/anchor projection, JSON shape, and conversion-failure logging. |
| src/test/java/com/williamcallahan/javachat/service/QdrantListenableFutureBridgeTest.java | Adds tests ensuring Guava↔JDK future completion and cancellation propagate correctly. |
| src/test/java/com/williamcallahan/javachat/service/HybridSearchServiceTest.java | Expands coverage for shared deadline budgeting, sparse citation query shape, and strictness guarantees. |
| src/test/java/com/williamcallahan/javachat/service/GuidedTOCProviderTest.java | Switches expected doc-set list to the canonical official identity projection. |
| src/test/java/com/williamcallahan/javachat/service/GuidedLearningServiceCitationTest.java | Updates guided-learning citation mocks/verifications to use discoverCitations. |
| src/test/java/com/williamcallahan/javachat/service/ChatServiceTest.java | Adds test ensuring chat citation lookup stays isolated from full retrieval/reranking. |
| src/test/java/com/williamcallahan/javachat/config/DocsSourceRegistryTest.java | Adds tests for official identity projection and filtering out non-official sources. |
| src/main/java/com/williamcallahan/javachat/service/RetrievalService.java | Replaces citation discovery with sparse-only discoverCitations and splits URL fragments into url + anchor. |
| src/main/java/com/williamcallahan/javachat/service/QdrantListenableFutureBridge.java | Propagates cancellation from CompletableFuture to the underlying Qdrant Guava future. |
| src/main/java/com/williamcallahan/javachat/service/HybridSearchService.java | Adds sparse-only citation search, shared fan-out deadline, and duration-based Qdrant query dispatch. |
| src/main/java/com/williamcallahan/javachat/service/GuidedTOCProvider.java | Uses the canonical official identity projection instead of re-deriving it locally. |
| src/main/java/com/williamcallahan/javachat/service/GuidedLearningService.java | Switches lesson citation lookup to discoverCitations. |
| src/main/java/com/williamcallahan/javachat/service/ChatService.java | Switches chat citation lookup to official sparse citation discovery. |
| src/main/java/com/williamcallahan/javachat/config/DocsSourceRegistry.java | Adds officialDocumentationSourceIdentities() and a canonical projection helper for official sources. |
| frontend/src/lib/utils/url.ts | Adds citationUrlIdentity() and updates citation deduping to respect case-sensitive anchors. |
| frontend/src/lib/utils/url.test.ts | Adds tests for encoded-anchor preservation and base-only normalization in citation identities. |
| frontend/src/lib/services/pageMetadata.ts | Adds nested route recovery logic to map unsupported lesson descendants back to canonical /learn. |
| frontend/src/lib/components/LearnView.test.ts | Adds mobile guided-chat focus restoration tests for post-stream completion scenarios. |
| frontend/src/lib/components/LearnView.svelte | Reserves mobile reading inset clearance so the lesson FAB/chat action doesn’t overlap content. |
| frontend/src/lib/components/LearnView.ownership.test.ts | Asserts the mobile reading inset clearance is present in the LearnView source. |
| frontend/src/lib/components/CitationPanel.test.ts | Verifies anchor-sensitive deduplication and correct link rendering for case/encoding edge cases. |
| frontend/src/lib/components/CitationPanel.svelte | Uses anchor-aware identity for keyed rendering and link building via shared URL utility. |
| frontend/src/lib/components/ChatView.test.ts | Adds chat input focus restoration tests after streaming completes without stealing external focus. |
| frontend/src/lib/components/ChatInput.svelte | Implements submission-owned focus restoration on disabled→enabled transitions via a form submit flow. |
| frontend/src/App.test.ts | Adds tests for recovering unsupported learn/guided descendant paths to canonical /learn. |
| frontend/src/App.svelte | Implements route recovery on initial load and popstate navigation using canonical recovery paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/lib/components/LearnView.ownership.test.ts (1)
161-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify the element presence assertion.
Hello there! 🌟 It looks like we're manually checking for the element's presence and then making a slightly tautological assertion (checking if an element selected by its class actually has that exact class!).
Let's learn a fun testing tidbit: the
jest-domlibrary gives us the super handytoBeInTheDocument()matcher! It will automatically fail with a helpful error message if the element turns out to benull, saving us from writing manualifchecks and custom errors. We can make this wonderfully concise! 🪄As per coding guidelines, use the simplest solution that works; achieve by removing, not adding (KISS principle).
🪄 Proposed magical simplification
- const lessonContentPanel =- learnView.container.querySelector<HTMLElement>(".lesson-content-panel");- if (!lessonContentPanel) {- throw new Error("Expected a scrollable lesson content panel");- }-- expect(lessonContentPanel).toHaveClass("lesson-content-panel");+ expect(learnView.container.querySelector(".lesson-content-panel")).toBeInTheDocument();🤖 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 `@frontend/src/lib/components/LearnView.ownership.test.ts` around lines 161 - 167, In the lessonContentPanel assertion within the ownership test, remove the manual null check and tautological toHaveClass assertion, and instead assert the queried element with jest-dom’s toBeInTheDocument matcher.Source: Coding guidelines
🤖 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 `@frontend/src/lib/components/ChatInput.svelte`:
- Around line 69-74: Update handleKeyDown so Enter submission is skipped while
the keyboard event indicates active IME composition, while preserving the
existing non-Shift Enter behavior otherwise. Add a regression test covering an
Enter keydown with isComposing enabled and verify submitMessage is not called.
In `@src/main/java/com/williamcallahan/javachat/service/ChatService.java`:
- Around line 85-87: Update ChatService’s citation flow and
GuidedLearningService’s lesson-citation flow to preserve
CitationOutcome.failedConversionCount() instead of returning only citations.
Apply the same outcome contract in both listed service locations, propagating
the complete outcome or raising the established typed failure so partial
conversion is treated as a strict failure.
In `@src/main/java/com/williamcallahan/javachat/service/HybridSearchService.java`:
- Around line 421-426: Update the InterruptedException handling in the hybrid
search flow to propagate a typed cancellation/retrieval exception instead of
recording a partial collection failure and continuing. Preserve interrupt
status, cancel every pending query future rather than only
collectionQueryFuture, then throw immediately so non-strict mode cannot return
documents after interruption.
---
Nitpick comments:
In `@frontend/src/lib/components/LearnView.ownership.test.ts`:
- Around line 161-167: In the lessonContentPanel assertion within the ownership
test, remove the manual null check and tautological toHaveClass assertion, and
instead assert the queried element with jest-dom’s toBeInTheDocument matcher.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d2ef485-5943-4ed8-8ba2-3fcbfb6b32eb
📒 Files selected for processing (27)
frontend/src/App.sveltefrontend/src/App.test.tsfrontend/src/lib/components/ChatInput.sveltefrontend/src/lib/components/ChatView.test.tsfrontend/src/lib/components/CitationPanel.sveltefrontend/src/lib/components/CitationPanel.test.tsfrontend/src/lib/components/LearnView.ownership.test.tsfrontend/src/lib/components/LearnView.sveltefrontend/src/lib/components/LearnView.test.tsfrontend/src/lib/services/pageMetadata.tsfrontend/src/lib/utils/url.test.tsfrontend/src/lib/utils/url.tssrc/main/java/com/williamcallahan/javachat/config/DocsSourceRegistry.javasrc/main/java/com/williamcallahan/javachat/service/ChatService.javasrc/main/java/com/williamcallahan/javachat/service/GuidedLearningService.javasrc/main/java/com/williamcallahan/javachat/service/GuidedTOCProvider.javasrc/main/java/com/williamcallahan/javachat/service/HybridSearchService.javasrc/main/java/com/williamcallahan/javachat/service/QdrantListenableFutureBridge.javasrc/main/java/com/williamcallahan/javachat/service/RetrievalService.javasrc/test/java/com/williamcallahan/javachat/config/DocsSourceRegistryTest.javasrc/test/java/com/williamcallahan/javachat/service/ChatServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/GuidedLearningServiceCitationTest.javasrc/test/java/com/williamcallahan/javachat/service/GuidedTOCProviderTest.javasrc/test/java/com/williamcallahan/javachat/service/HybridSearchServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/QdrantListenableFutureBridgeTest.javasrc/test/java/com/williamcallahan/javachat/service/RetrievalServiceCitationTest.javasrc/test/java/com/williamcallahan/javachat/service/RetrievalServiceTest.java
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:0ec55c78b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Summary
/learn, and reserve mobile reading space above the lesson chat actionProduction findings resolved
urlwhileanchorremained emptyValidation
make lintpassed, including ast-grep, Svelte checks, PMD, and SpotBugsmake buildpassed