feat: structured prompt truncation and streaming state refactor - #8
Merged
Conversation
Hover-based UI elements (arrows in citations and lesson cards) were using width-based media queries (@media max-width: 640px) which incorrectly assumes small screens = touch and large screens = hover. This fails on tablets like iPad where the screen is large but input is touch-based, causing arrows to appear on tap and persist in a "stuck" hover state. Switch to capability-based detection (@media (hover: hover) and (pointer: fine)) which directly queries whether the device supports hover input, matching the gold-standard pattern already used in MessageBubble.svelte. - CitationPanel: Arrow now visible by default (opacity 0.5), hidden only on hover-capable devices, with hover animation in capability media query - LearnView: Lesson card arrows visible by default (opacity 0.6), hover effects (card transform, arrow animation) wrapped in capability media query - Remove redundant width-based arrow opacity rule from CitationPanel
…r components LearnView.svelte (1356 LOC) and ChatView.svelte contained duplicated patterns for rendering message lists with streaming indicators. The mobile chat drawer markup and CSS (~200 lines) was also inline in LearnView. This extraction reduces duplication and shrinks the LearnView monolith to 1069 LOC while creating reusable components following clean architecture principles. StreamingMessagesList encapsulates: - Message list iteration with configurable rendering via Svelte 5 snippets - In-progress streaming content as a live MessageBubble - ThinkingIndicator when streaming but no content yet - CSS custom property for flexible gap spacing MobileChatDrawer encapsulates: - Floating action button with message count badge and streaming indicator - Full-screen backdrop with fade animation - Slide-up drawer with header, clear/close actions, and safe area padding - Integrates StreamingMessagesList for message rendering - Create StreamingMessagesList.svelte (66 lines) with messageRenderer snippet - Create MobileChatDrawer.svelte (357 lines) with getMessagesContainer() export - Update ChatView to use StreamingMessagesList with citation wrapper snippet - Update LearnView to use both new components, removing ~287 lines - Add StreamingChatFields type to stream-types.ts for documentation
…erations The renderMarkdown function was using document.createElement in $derived contexts, causing hydration mismatches when lesson content loaded on the left panel and triggered re-renders of the right-side chat. This splits the function into: - parseMarkdown(): Pure string transformation, SSR-safe, for use in $derived - applyJavaLanguageDetection(): DOM-based, client-side only, for use in $effect Additionally fixes escapeHtml to use pure string operations instead of DOM APIs, extracts magic literals to named constants, and adds defensive null validation. - Split renderMarkdown into parseMarkdown + applyJavaLanguageDetection - Extract JAVA_KEYWORDS, JAVA_LANGUAGE_CLASS, UNMARKED_CODE_SELECTOR constants - Add try-catch with error logging to parseMarkdown - Add container validation with dev warning to applyJavaLanguageDetection - Convert escapeHtml from DOM-based to pure string replacement - Update LearnView and MessageBubble to use new SSR-safe pattern
Adds testing infrastructure for the Svelte frontend with vitest, jsdom, and @testing-library/svelte. Includes unit tests for core utilities that were refactored for SSR safety. - Configure vitest with jsdom environment and Svelte plugin - Add test setup with jest-dom matchers and matchMedia mock - Add tests for parseMarkdown, applyJavaLanguageDetection, escapeHtml - Add tests for scroll utilities (isNearBottom, scrollToBottom) - Add tests for URL utilities (sanitizeUrl, buildFullUrl, getCitationType, etc.)
…cation Introduces a domain model for structured prompts that enables segment-by-segment truncation rather than character-by-character. When prompts exceed model limits, the truncator can drop lowest-priority segments (older context docs, conversation turns) while preserving semantic boundaries and required segments. Domain types: - PromptSegment: Base sealed interface with token estimate and priority - SystemSegment, ContextDocumentSegment, ConversationTurnSegment, CurrentQuerySegment - StructuredPrompt: Composite with serialization and truncation support Application service: - PromptTruncator: Truncates structured prompts to fit model token limits Integrations: - ChatService.buildStructuredPromptWithContextAndGuidance for guided learning - GuidedLearningService.buildStructuredGuidedPromptWithContext - OpenAIStreamingService constructor accepts PromptTruncator - Add domain/prompt package with segment types and StructuredPrompt - Add application/prompt/PromptTruncator service - Deprecate string-based buildGuidedPromptWithContext in favor of structured - Update OpenAIStreamingService tests for new constructor signature
…arning controllers Completes the structured prompt integration by updating the controllers to use the new structure-aware streaming API. Prompts are now truncated by dropping low-priority segments (older context docs, conversation history) rather than arbitrary character boundaries. - ChatController: use buildStructuredPromptWithContextOutcome and streamResponse(StructuredPrompt) - GuidedLearningController: use buildStructuredGuidedPromptWithContext and streamResponse(StructuredPrompt)
…on utility ChatView duplicated streaming state management patterns (isStreaming, content, status, timers) that will also be needed by LearnView. This extracts a reusable composable that encapsulates the reactive state and lifecycle methods. createStreamingState composable provides: - Reactive getters for isStreaming, streamingContent, statusMessage, statusDetails - Action methods: startStream, appendContent, updateStatus, finishStream, reset - Configurable status persistence delay (ChatView uses 800ms, LearnView uses 0) - Automatic timer cleanup via cleanup() function Also extracts generateSessionId utility for consistent session ID generation across views, with domain-specific prefixes. - Create createStreamingState.svelte.ts composable with typed interface - Create session.ts with generateSessionId utility - Refactor ChatView to use composable, reducing ~50 lines of boilerplate - Add prefers-reduced-motion media query for scroll-behavior
…lity
Applies the same DRY refactoring to LearnView that was done for ChatView,
using createStreamingState composable with immediate status clearing (0ms delay)
and generateSessionId for consistent session ID generation.
- Replace inline streaming state with createStreamingState() composable
- Use generateSessionId('guided') for session ID
- Reduce boilerplate by ~40 linesThe structured prompt system (StructuredPrompt, StructuredPromptOutcome) now handles all streaming paths with intelligent segment-based truncation. The legacy string-based methods were deprecated shims with no external callers, violating AB4 (delete unused code) and creating maintenance burden. Removed methods and types: - appendContextDocs() - duplicated by buildContextSegments() - buildPromptFromMessages() - replaced by StructuredPrompt.render() - buildPromptWithContext() (2 overloads) - no external callers - buildPromptWithContextOutcome() - no external callers - buildPromptWithContextAndGuidance() - no external callers - ChatPromptOutcome record - superseded by StructuredPromptOutcome File reduced from 540 to 364 lines (32% reduction).
…ence
Replace $lib alias imports with relative paths for consistency with the
rest of the codebase. Also fixes scrollToBottom() to check user's
prefers-reduced-motion setting, using instant scrolling ('auto') when
reduced motion is preferred instead of forcing smooth animation.
- ChatView, LearnView: change $lib/composables to ../composables
- createStreamingState: change $lib/services to ../services
- scroll.ts: check matchMedia('(prefers-reduced-motion: reduce)')
- scroll.test.ts: add mockMatchMedia helper and motion preference testsThe two-column layout in LearnView lost the chat panel's pinned position when left column content grew taller than the viewport. The cascade of min-height: 100vh/100dvh on body, #app, and .app-shell allowed parent containers to expand beyond the viewport, causing the CSS Grid row to expand and the chat panel to scroll out of view. Changing from min-height to fixed height at the root level creates a hard viewport constraint that forces all overflow to the designated scroll containers (.lesson-content-panel and .messages-container). - body, #app, .app-shell: change min-height to height with overflow: hidden - .lesson-layout: add max-height: 100% as extra protection against expansion - Update @supports queries to use height instead of min-height for 100dvh
Contributor
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds frontend testing and a streaming UI/state refactor (new composable, streaming components, session util, SSR-safe markdown + Java detection, motion-aware scrolling), and a backend structured-prompt model with PromptTruncator used by streaming services; tests and config added across both layers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatService
participant PromptTruncator
participant OpenAIStreamingService
participant OpenAI
Client->>ChatService: buildStructuredPromptWithContextOutcome(history, query)
ChatService->>ChatService: build segments (system, context, history, query)
ChatService-->>Client: StructuredPromptOutcome
Client->>OpenAIStreamingService: streamResponse(structuredPrompt, temperature)
OpenAIStreamingService->>PromptTruncator: truncate(structuredPrompt, maxTokens, isGpt5)
PromptTruncator->>PromptTruncator: preserve CRITICAL (system) & HIGH (current query)
PromptTruncator->>PromptTruncator: trim LOW/MEDIUM (context/docs, turns) newest-first
PromptTruncator-->>OpenAIStreamingService: TruncatedPrompt
OpenAIStreamingService->>OpenAI: POST streaming request with truncated prompt
OpenAI-->>OpenAIStreamingService: stream chunks (SSE)
OpenAIStreamingService-->>Client: relay SSE events (chunks, citations, status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This pull request introduces a major refactoring of both backend prompt handling and frontend chat streaming, implementing structured prompt truncation and improving UI modularity. The changes enable intelligent, segment-based prompt truncation that preserves semantic boundaries, along with improved accessibility and SSR-compatibility in the frontend.
Changes:
- Introduced structured prompt domain model with priority-based truncation in Java backend
- Refactored frontend chat views to use composable streaming state management
- Enhanced markdown rendering with SSR-safe parsing and client-side language detection
- Improved accessibility with reduced-motion support and hover capability detection
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/williamcallahan/javachat/domain/prompt/* | New domain model for typed prompt segments with priorities |
| src/main/java/com/williamcallahan/javachat/application/prompt/PromptTruncator.java | Structure-aware prompt truncation logic (has priority order issue) |
| src/main/java/com/williamcallahan/javachat/service/ChatService.java | Refactored to build structured prompts instead of strings |
| src/main/java/com/williamcallahan/javachat/service/OpenAIStreamingService.java | Added structured prompt streaming with deprecation of string-based method |
| src/main/java/com/williamcallahan/javachat/web/*Controller.java | Updated controllers to use structured prompts |
| src/test/java/com/williamcallahan/javachat/application/prompt/PromptTruncatorTest.java | Comprehensive tests for truncation logic (misleading test name) |
| frontend/src/lib/composables/createStreamingState.svelte.ts | New composable for streaming state management (has $effect cleanup issue) |
| frontend/src/lib/services/markdown.ts | Split into SSR-safe parsing and client-side DOM manipulation |
| frontend/src/lib/components/ChatView.svelte | Refactored to use StreamingMessagesList and createStreamingState |
| frontend/src/lib/components/LearnView.svelte | Similar refactoring with mobile drawer component |
| frontend/src/lib/components/StreamingMessagesList.svelte | New reusable component for message rendering |
| frontend/src/lib/components/MobileChatDrawer.svelte | New component for mobile chat UI |
| frontend/src/lib/components/CitationPanel.svelte | Deterministic ARIA IDs and hover capability detection |
| frontend/src/lib/utils/scroll.ts | Added prefers-reduced-motion support |
| frontend/src/styles/global.css | Changed to fixed height with overflow hidden (potential accessibility concern) |
| frontend/package.json | Added Vitest and Testing Library dependencies |
| frontend/vitest.config.ts | Vitest configuration for component testing |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 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.
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.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/lib/services/markdown.ts (1)
210-236: DOMPurify needs a DOM on the server—fix SSR support.DOMPurify 3.3.1 requires a DOM implementation (
window/jsdom) to run in Node.js. Your current setup imports it directly with no isomorphic wrapper, so whenparseMarkdownruns server-side,DOMPurify.sanitize()will throw, and the catch block returns''—silently blanking your SSR output and causing hydration mismatch.The good news? There's a straightforward fix: swap
dompurifyforisomorphic-dompurify(it's the standard pattern for this exact scenario). It wires DOMPurify with JSDOM automatically, sosanitize()works the same way on server and client. Alternatively, you could manually initialize DOMPurify with a JSDOM-backed DOM, but isomorphic-dompurify saves you that plumbing.Since
jsdomis already in your devDependencies, isomorphic-dompurify will pick it up seamlessly.
🤖 Fix all issues with AI agents
In `@frontend/package.json`:
- Around line 10-26: Add an "engines" field to frontend/package.json declaring
the minimum Node versions required by the test toolchain: insert an "engines"
object (key "node") alongside the existing top-level fields (near "scripts" and
"devDependencies") with the range ">=20.19.0 || >=22.12.0 || >=24.0.0" so
package managers and CI can enforce the Node version constraint.
In `@frontend/src/lib/components/MobileChatDrawer.svelte`:
- Around line 90-95: The Clear chat button (button with class
"drawer-action-btn" and onclick={onClear} in MobileChatDrawer.svelte) is
icon-only and uses title for tooltip but lacks an accessible label; add an
aria-label (e.g., aria-label="Clear chat") to the button element so screen
readers announce its purpose while keeping the existing title and SVG unchanged.
In `@frontend/src/lib/components/StreamingMessagesList.svelte`:
- Around line 41-47: The current {`#each` messages as message, messageIndex
(message.timestamp)} key can collide and using messageIndex in a composite key
is unstable; add a stable id to the ChatMessage model and use that as the Svelte
each key instead: update the ChatMessage shape to include a unique id when
messages are created/received, ensure any factories/parsers that construct
messages populate message.id, and change the each block key to use message.id
(referencing the StreamingMessagesList.svelte each block, messageRenderer and
MessageBubble usage) rather than timestamp or messageIndex.
In `@frontend/src/lib/composables/createStreamingState.svelte.ts`:
- Line 58: Update the documentation example so the $effect actually returns the
cleanup function instead of calling it or omitting the return; replace the
incorrect usage with an explicit return (e.g. use $effect(() =>
streaming.cleanup) or $effect(() => { return streaming.cleanup }) so the
streaming.cleanup function is returned as the cleanup callback (do not call
streaming.cleanup()).
In
`@src/main/java/com/williamcallahan/javachat/application/prompt/PromptTruncator.java`:
- Around line 52-68: The truncate method currently returns a TruncatedPrompt
with wasTruncated=true whenever system+query exceed the limit even if the
original StructuredPrompt already had no context or history; change truncate (in
PromptTruncator.truncate) to compute whether any content will actually be
removed by checking prompt.context() and prompt.history() before setting
wasTruncated, i.e., build the minimalPrompt as before but set the boolean to
true only if prompt.context() or prompt.history() are non-empty (otherwise set
wasTruncated=false) when constructing the TruncatedPrompt.
- Around line 145-188: fitDocumentsNewestFirst assumes the most relevant
documents are at the end of the input list, but rerankers typically place
highest-relevance items first; this causes the truncator to drop the best docs.
Fix by making the truncator order-agnostic: in
PromptTruncator.fitDocumentsNewestFirst (or rename to reflect behavior) either
reverse the input when the list is in descending relevance or change the
selection loop to iterate from the start of docs and pick documents until
availableTokens is exhausted, then reindex as you already do; additionally
ensure callers (RetrievalService/ChatService.buildContextSegments) produce and
document a consistent ordering or adjust tests (e.g.,
reindexesContextDocumentsAfterTruncation) to use actual reranked output to
validate the corrected behavior.
In
`@src/main/java/com/williamcallahan/javachat/domain/prompt/ConversationTurnSegment.java`:
- Around line 34-44: The constructor for ConversationTurnSegment currently only
checks role for null; update the constructor to validate that role equals "user"
or "assistant" (or the corresponding enum/constants if used) and throw an
IllegalArgumentException with a clear message when it contains any other value;
keep the existing null checks for messageText and the estimatedTokens >= 0 guard
(setting to 0 if negative) in the ConversationTurnSegment constructor to enforce
the invariant early.
In
`@src/main/java/com/williamcallahan/javachat/domain/prompt/CurrentQuerySegment.java`:
- Around line 3-9: Update the Javadoc summary for the CurrentQuerySegment class
so the first line is a complete present‑tense sentence; locate the class-level
comment above the CurrentQuerySegment declaration and change the opening summary
line to something like "Represents the current user query segment containing the
active question being answered." while keeping the existing paragraph that
follows.
In `@src/main/java/com/williamcallahan/javachat/domain/prompt/SystemSegment.java`:
- Around line 31-38: Add Javadoc to the public overrides in SystemSegment:
annotate the content() and priority() methods with Javadoc using {`@inheritDoc`}
to inherit documentation from the interface/parent rather than leaving them
undocumented; update the SystemSegment class so the content() and priority()
methods include a brief Javadoc block (e.g., {`@inheritDoc`}) above each method
signature that references PromptSegmentPriority.CRITICAL where applicable.
In
`@src/test/java/com/williamcallahan/javachat/application/prompt/PromptTruncatorTest.java`:
- Around line 58-66: Update the inline token-count comment to match the actual
call to truncator.truncate(prompt, 400, false): change the earlier "Limit 350"
reference and any derived math to reflect a 400-token limit so the arithmetic
aligns with the test; locate the block around the truncate invocation and the
assertions involving PromptTruncator.TruncatedPrompt (result.wasTruncated() and
result.contextDocumentCount()) and correct the reserved/available token
calculations in the comments to 400 so readers don't have to re-derive the
numbers.
♻️ Duplicate comments (2)
src/main/java/com/williamcallahan/javachat/domain/prompt/ContextDocumentSegment.java (1)
45-53: Same Javadoc requirement for public overrides here.
Consider/** {@inheritdoc} */oncontent()andpriority()like the other segments.src/main/java/com/williamcallahan/javachat/domain/prompt/ConversationTurnSegment.java (1)
46-57: Same public-override Javadoc note here too./** {@inheritdoc} */keeps things tidy and compliant.
🧹 Nitpick comments (6)
src/main/java/com/williamcallahan/javachat/web/ChatController.java (1)
128-136: Avoid prompt work when streaming is unavailable.
Right now the structured prompt is built before theisAvailable()guard, so we still do retrieval/prompt assembly even when we’ll return an error. Moving it inside theiftrims work and side effects.💡 Suggested tweak
- // Build structured prompt for intelligent truncation- // Pass model hint to optimize RAG for token-constrained models- ChatService.StructuredPromptOutcome promptOutcome =- chatService.buildStructuredPromptWithContextOutcome(history, userQuery, ModelConfiguration.DEFAULT_MODEL);-- // Use OpenAI streaming only (legacy fallback removed)- if (openAIStreamingService.isAvailable()) {+ // Use OpenAI streaming only (legacy fallback removed)+ if (openAIStreamingService.isAvailable()) {+ // Build structured prompt for intelligent truncation+ // Pass model hint to optimize RAG for token-constrained models+ ChatService.StructuredPromptOutcome promptOutcome =+ chatService.buildStructuredPromptWithContextOutcome(history, userQuery, ModelConfiguration.DEFAULT_MODEL); PIPELINE_LOG.info("[{}] Using OpenAI Java SDK for streaming (structured prompt)", requestToken);src/main/java/com/williamcallahan/javachat/service/ChatService.java (1)
293-299: Token estimation looks reasonable!The ~4 chars/token heuristic is a common approximation for English text with GPT-style tokenizers. The
+1ensures you never underestimate for short strings.One small refinement to consider: extracting
4to a named constant likeCHARS_PER_TOKEN_ESTIMATEwould make the intent even clearer and satisfy the "no magic literals" guideline. But since it's documented in the Javadoc, this is a minor polish item.🔧 Optional: Extract constant
+ /** Conservative estimate of characters per token for English text. */+ private static final int CHARS_PER_TOKEN_ESTIMATE = 4;+ private int estimateTokens(String text) { if (text == null || text.isEmpty()) { return 0; } - // Conservative: ~4 chars per token, add 1 for rounding- return (text.length() / 4) + 1;+ return (text.length() / CHARS_PER_TOKEN_ESTIMATE) + 1; }frontend/vitest.config.ts (1)
4-17: Consider excluding*.spec.*from coverage too.Right now Line 14–15 excludes only
*.test.ts. If you add or already have*.spec.ts, they’ll count toward coverage. Quick tweak below keeps coverage focused on app code.♻️ Proposed tweak
coverage: { provider: 'v8', reporter: ['text', 'html'], include: ['src/lib/**/*.{ts,svelte}'], - exclude: ['src/lib/**/*.test.ts', 'src/test/**']+ exclude: ['src/lib/**/*.{test,spec}.{ts,svelte}', 'src/test/**'] }frontend/src/lib/utils/url.test.ts (1)
46-54: Duplicate test detected!The test on Lines 46-48 and the test on Lines 50-54 both verify the same behavior—appending an anchor with a hash separator. The second test even has a comment acknowledging this. Consider consolidating these into a single, more comprehensive test or removing the duplicate.
Fun fact: duplicate tests are like eating the same cookie twice—still delicious, but maybe a bit redundant! 🍪
♻️ Suggested consolidation
- it('appends anchor with hash', () => {- expect(buildFullUrl('https://example.com', 'section')).toBe('https://example.com#section')- })-- it('handles anchor - appends with hash separator', () => {- // Note: buildFullUrl always prepends # to anchor, so `#section` becomes ##section- // Callers should strip # from anchors before passing- expect(buildFullUrl('https://example.com', 'section')).toBe('https://example.com#section')- })+ it('appends anchor with hash separator', () => {+ // Note: buildFullUrl always prepends # to anchor+ // Callers should strip # from anchors before passing+ expect(buildFullUrl('https://example.com', 'section')).toBe('https://example.com#section')+ })frontend/src/lib/services/markdown.test.ts (1)
25-30: XSS test could be more precise.The assertion
expect(result).not.toContain('alert')on Line 29 might be overly broad. If the sanitizer converts<script>alert("xss")</script>to escaped text, the word "alert" could still appear in the output. The<script>tag check on Line 28 is the critical one.Consider checking for the executable form rather than the word itself:
♻️ More precise assertion
it('sanitizes dangerous HTML', () => { const markdown = '<script>alert("xss")</script>' const result = parseMarkdown(markdown) expect(result).not.toContain('<script>') - expect(result).not.toContain('alert')+ // Script tags should be stripped entirely by DOMPurify+ expect(result).not.toMatch(/<script[^>]*>/i) })frontend/src/lib/components/MobileChatDrawer.svelte (1)
81-88: Consider aria-modal and a labeled dialog header.
Small a11y win:aria-modal="true"plusaria-labelledbymakes the dialog context clearer for screen readers (and future focus-trap hooks).💡 Suggested tweak
- <div class="chat-drawer" role="dialog" aria-label="Lesson chat">+ <div class="chat-drawer" role="dialog" aria-modal="true" aria-labelledby="chat-drawer-title"> ... - <span>{title}</span>+ <span id="chat-drawer-title">{title}</span>
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.
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.
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.
Uh oh!
There was an error while loading. Please reload this page.
…t behavior
The $effect hook in Svelte 5 requires returning the cleanup function to register
it for component unmount. The previous syntax `$effect(() => streaming.cleanup)`
only evaluated the cleanup reference without registering it. The correct syntax
returns the function: `$effect(() => { return streaming.cleanup })`.The documentation example showed incorrect Svelte 5 syntax for cleanup registration. Updated to show correct pattern: return the cleanup function from the $effect callback rather than just referencing it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:089faeb6de
ℹ️ 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.
Uh oh!
There was an error while loading. Please reload this page.
railway-appBottemporarily deployed
to
insightful-intuition / production
January 25, 2026 20:02
Inactive
…elevant The fitDocumentsNewestFirst method incorrectly reversed the document list and kept documents from the end, assuming they were highest-scored. However, the reranker orders documents with most relevant first, so the truncator was actually dropping the best documents. Renamed to fitDocumentsByRelevance and changed to iterate from the start, keeping documents until the token budget is exhausted. This preserves the most relevant documents when truncation is needed.
…ment order Updated reindexesContextDocumentsAfterTruncation test to verify that the truncator keeps the first (most relevant) documents rather than the last. The test now asserts url1 and url2 are preserved while url3 is dropped.
railway-appBottemporarily deployed
to
insightful-intuition / production
January 25, 2026 20:10
Inactive
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 17, 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 freeto 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.
Full-stack improvements to prompt handling and chat streaming architecture.
New Features
Refactoring
createStreamingStatecomposable for reusable streaming lifecycle management across ChatView and LearnView, with configurable status persistencerenderMarkdowninto pureparseMarkdown(SSR-safe) andapplyJavaLanguageDetection(client-side DOM) to fix hydration mismatchesBug Fixes
min-heightto fixedheightwithoverflow: hidden@media (hover: hover) and (pointer: fine)for proper touch device handlingprefers-reduced-motionfor scroll animationsOther Changes