fix(chat): expanded sources stay visible and embedding warm-up probes keep cadence without pipeline logs#54
Merged
Merged
Conversation
… pipeline logs fixedRate replaces fixedDelay so a slow cold-start probe cannot push the next probe past the provider's idle-unload TTL, and probe latency is measured with the monotonic nanoTime clock. The probe now calls a dedicated EmbeddingClient.warmUp() whose internal embed call is a self-invocation outside the AOP proxy, so the RAG pipeline aspect's 'STEP 1: EMBEDDING GENERATION' logs stay scoped to real requests instead of firing every 4 minutes.
…ontainers The sources disclosure trigger sits at the very bottom of every scroll container it appears in (chat messages, lesson content panel, mobile chat drawer), and the citation list expands downward. Browsers never auto-scroll to reveal newly inserted content below the fold, so the expanded list was partially clipped on desktop and rendered entirely off-screen on mobile, where tapping the trigger appeared to do nothing at all. Reveal the list with scrollIntoView(block: 'nearest') once it mounts while expanded, and give it scroll-margin-bottom clearance because the scroll fires while the slide-down entry animation still holds the list 4px above its settled position. - Bind the expanded list element and scroll it into view from an $effect - Add scroll-margin-bottom to out-clear the translateY(-4px) entry offset - Cover expand, reveal-scroll, and collapse paths in CitationPanel tests - Polyfill scrollIntoView in the jsdom test setup alongside scrollTo
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughFrontend: expanded citation lists now bind their UL and call scrollIntoView({ block: 'nearest' }) with a scroll-margin-bottom offset; tests and a jsdom polyfill added. Header tabs gained aria-labels and tests. Backend: added EmbeddingClient.warmUp(), refactored keep-alive to fixed-rate nano timing and updated clients/tests. Build: SOURCE_COMMIT flows into Gradle, Docker, and actuator info. ChangesCitation Panel Scroll-Into-View
Embedding Keep-Alive Refactoring
Build and Runtime Metadata
Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant KeepAlive as EmbeddingModelKeepAlive
participant EmbeddingClient
participant EmbeddingService
Scheduler->>KeepAlive: scheduled fixed-rate probe
KeepAlive->>EmbeddingClient: warmUp()
EmbeddingClient->>EmbeddingService: provider embedding probe request
EmbeddingService-->>EmbeddingClient: probe response / error
EmbeddingClient-->>KeepAlive: success or EmbeddingServiceUnavailableException
KeepAlive->>KeepAlive: measure duration via System.nanoTime()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
coderabbitai
Bot
added
enhancement
There was a problem hiding this comment.
Pull request overview
This PR improves chat UX and operational robustness by ensuring expanded citation lists remain visible within scroll containers and by making embedding keep-alive probes run on a fixed cadence while avoiding being logged as normal RAG pipeline embedding work.
Changes:
- Frontend: Scroll expanded citation source lists into view and add a
scrollIntoViewjsdom polyfill plus component tests covering expand/scroll/collapse behavior. - Backend: Switch embedding keep-alive scheduling from
fixedDelaytofixedRate, measure probe latency with monotonic time, and route keep-alive probes through a dedicatedEmbeddingClient.warmUp()path. - Tests: Update/add tests to validate citation expansion behavior and ensure keep-alive probes still trigger embedding activity.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAlive.java | Moves keep-alive probes to fixed-rate scheduling, monotonic timing, and calls EmbeddingClient.warmUp(). |
| src/main/java/com/williamcallahan/javachat/service/EmbeddingClient.java | Adds a warmUp() method intended to issue minimal embedding probes without normal pipeline logging. |
| src/test/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAliveTest.java | Adjusts test expectations/documentation around keep-alive probe behavior. |
| frontend/src/lib/components/CitationPanel.svelte | Scrolls the expanded citation list into view and adds scroll margin for animation clearance. |
| frontend/src/lib/components/CitationPanel.test.ts | Adds Vitest coverage for expand, scroll reveal, and collapse behavior. |
| frontend/src/test/setup.ts | Adds jsdom scrollIntoView polyfill for component tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1c461015d
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/williamcallahan/javachat/service/EmbeddingClient.java (1)
41-54: 💤 Low valueLovely documentation! The AOP proxy bypass rationale is crystal clear. 🎯
Your Javadoc beautifully explains why warmUp() exists as a separate method—the self-invocation detail shows deep understanding of Spring's proxy mechanics. This kind of "why" documentation is exactly what makes code maintainable long-term.
One tiny style note: the inline string
"embedding model warm-up probe"at line 53 could technically be extracted to a constant per the "no inline strings" guideline. But since it's used once in a crystal-clear context, the practical value is minimal. Totally your call whether to make it:String WARMUP_PROBE_TEXT = "embedding model warm-up probe";Either way, this is clean, thoughtful code! The fixed-rate scheduling change in the other file pairs perfectly with this abstraction.
🤖 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 `@src/main/java/com/williamcallahan/javachat/service/EmbeddingClient.java` around lines 41 - 54, Extract the inline warm-up probe string used in EmbeddingClient.warmUp() into a named constant (e.g., WARMUP_PROBE_TEXT) to satisfy the "no inline strings" guideline; update warmUp() to call embed(List.of(WARMUP_PROBE_TEXT)) and declare the constant near the interface methods (static final or equivalent visibility for interfaces) so the string is reusable and clearly identified.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.
Nitpick comments:
In `@src/main/java/com/williamcallahan/javachat/service/EmbeddingClient.java`:
- Around line 41-54: Extract the inline warm-up probe string used in
EmbeddingClient.warmUp() into a named constant (e.g., WARMUP_PROBE_TEXT) to
satisfy the "no inline strings" guideline; update warmUp() to call
embed(List.of(WARMUP_PROBE_TEXT)) and declare the constant near the interface
methods (static final or equivalent visibility for interfaces) so the string is
reusable and clearly identified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26d12430-56ff-4d64-aa44-ee81e950e558
📒 Files selected for processing (6)
frontend/src/lib/components/CitationPanel.sveltefrontend/src/lib/components/CitationPanel.test.tsfrontend/src/test/setup.tssrc/main/java/com/williamcallahan/javachat/service/EmbeddingClient.javasrc/main/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAlive.javasrc/test/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAliveTest.java
Mobile navigation collapsed to icon-only tabs without accessible names, and the dev deployment did not expose commit metadata for verification. - Add accessible names and regression coverage for the Chat and Learn tabs - Generate Spring Boot build info with the deployed source commit - Surface deployment commit details through actuator info
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
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
Expanded citation source lists now scroll into view inside chat containers, so users can see sources immediately after opening them. Embedding keep-alive probes now start on a fixed cadence and use provider-specific warm-up paths that avoid RAG pipeline logging, reducing cold-start risk without making scheduled probes look like user retrieval work.
Changes
Bug Fixes
CitationPanel.svelte).EmbeddingModelKeepAlive.keepEmbeddingModelWarm).embed(List)method (EmbeddingClient.warmUp,LocalEmbeddingClient.warmUp,OpenAiCompatibleEmbeddingClient.warmUp).Tests
scrollIntoViewpolyfill (CitationPanel.test.ts,frontend/src/test/setup.ts).embed(List)instead of the warm-up port (EmbeddingModelKeepAliveTest).Breaking Changes
None
Related Issues
None