feat(release): deadline-bounded retrieval with live progress streaming, resilient streaming chat UX, and Java 25 specification ingestion - #141
Merged
Merged
Conversation
Send retrieved reference text as user-role input so document instructions cannot outrank the learner while preserving application-owned guidance at developer authority.
Split packaged lessons on parsed Markdown headings so constrained models retain canonical material, fail explicitly when none fits, and keep stable section identities through provider truncation. Move retained-document citation selection to its service owners and require exact non-null stream identities so emitted citations match the prompt actually sent.
Reject SDK-wide reasoning effort values that GPT-5.4 does not support. Keep startup validation deterministic when a local dotenv file uses another model.
Delayed status cleanup left description and provider metadata behind, allowing later events to revive stale details. Clear the complete status state at the owning composable boundary. - Reset status text, description, and provider metadata together - Cover delayed cleanup followed by a later provider event
Zero-width joiner characters could make an otherwise empty streamed message appear visible. Keep visibility classification aligned with the backend text boundary. - Treat zero-width non-joiner and joiner characters as invisible - Cover whitespace-only messages containing both code points
Local development and ingestion entrypoints could signal processes they did not own, while PID-file checks still allowed concurrent ingestion races. Make ownership conflicts explicit and non-destructive. - Refuse occupied local ports instead of terminating listeners - Atomically reserve the ingestion PID path without signaling prior runs - Cover occupied ports, existing PID files, and concurrent PID claims
Redirect deduplication used the final URL for visitation but the requested alias for persistence, and alias floods could exceed the requested HTTP budget. Keep crawl and storage identity consistent. - Bound the crawl by actual fetch count - Persist, chunk, and replace documents under the verified final URL - Remove redirected vector aliases and cover canonical identity plus alias floods
Generic fence normalization misclassified literal closing braces, while the dependency text collector dropped inline code from citation labels. Route each behavior through its direct parser owner. - Extract proven enrichments before generic Markdown normalization - Recognize fenced enrichment endings only inside the enrichment parser - Preserve nested emphasis and inline code in citation titles
Javadoc anchors retain inner array dimensions when the outer array becomes varargs. Accept that canonical signature shape instead of rejecting it. - Preserve array suffixes before a terminal varargs suffix - Cover one- and two-dimensional array varargs anchors
Malformed Javadoc detail sections can lack DOM identifiers, violating the member-anchor invariant and dropping the entire extraction. Skip only the malformed section. - Require a non-blank DOM identifier before constructing an anchor - Preserve valid sibling member sections
Treat joiner-only output as empty at the backend boundary and surface unsupported Java documentation releases as actionable non-retryable guided-learning errors.
Read full persisted chunk hashes during audits, fail closed for unrecoverable legacy names, and skip malformed blank Javadoc member identifiers without losing valid siblings.
Continue directory ingestion after isolated extraction failures, stop on quarantine or dependency failures, and close both owned embedding clients while preserving shutdown diagnostics.
Assert a same-key caller enters the shared wait before its deadline and still fails within its own budget while the owning rerank remains blocked.
…ruption The pre-push hook runs make build && make test, which exhausted the macOS FSEvents stream and corrupted Gradle's VFS state, causing intermittent test-result write failures. - Set org.gradle.vfs.watch=false in gradle.properties. - Pass -Dorg.gradle.vfs.watch=false on every Make-driven Gradle invocation so a stale daemon or a global ~/.gradle/gradle.properties value cannot override the repo setting. - Run Java tests as --no-daemon cleanTest test so each pre-push test run starts from a clean VFS state and cannot inherit a corrupted daemon.
A dropped connection surfaced the raw browser TypeError text (Failed to fetch) in the assistant bubble with no retry affordance, inconsistent with server-sent error events that carry details and a retryable flag. Wraps fetch rejections and mid-stream read failures in a StreamFailureError with a friendly connection message, recovery details, and retryable: true, so every transport failure renders like a server-sent retryable error in both the chat and guided flows.
The QdrantSearchAdmission semaphore serialized every whole dense-search fan-out behind a single permit, so concurrent searches blocked each other even when the stage deadline left enough budget to dispatch them independently. The gate added a contention path with no correctness benefit: each search already derives its own query deadline and dispatches its per-collection futures under that budget. Drop the admission layer and inline the direct dispatch flow already used by the citation and scroll paths, so dense search reuses the same deadline-aware dispatch shape. Rewrite the admission-focused tests to assert that concurrent searches dispatch their fan-outs independently. - Delete QdrantSearchAdmission and its single-permit concurrency gate - Remove QdrantQueryExecutor.executeAdmitted and the LongFunction bridge - Inline direct dispatch in HybridSearchService dense-search path - Replace admission-saturation tests with independent-dispatch coverage
The chat bubble used the flex row's default min-content width, so an in-flow copy action inside the bubble forced it wider than the panel on narrow chat widths and clipped off-screen. Setting min-width: 0 lets the flex item shrink beneath its content's minimum so the bubble fits the panel and the copy action stays visible. - Add min-width: 0 to .message-bubble with rationale comment
Hybrid and citation queries enabled full payload transfer, so every candidate point shipped its entire stored document over the network even though QdrantScoredPointDocumentMapper only reads doc_content and the metadata fields. That duplicated the dense document text for every point on every retrieval and citation scroll. Select exactly the fields the mapper consumes via WithPayloadSelectorFactory.include, shared across the hybrid query, sparse citation query, and citation scroll paths. Update the search tests to assert the selected include-fields instead of the enable flag. - Add RETRIEVAL_PAYLOAD_FIELDS from QdrantPayloadFieldSchema - Replace setWithPayload(enable) with retrievalPayloadSelector() in all three request builders - Assert include-fields contain doc_content, url, title in tests
… cannot stall live requests The live and batch request launch pacers shared a single EmbeddingProviderCooldown instance, so a 429 Retry-After from the batch tier armed the same cooldown the live tier consulted. A batch ingest rate-limit then blocked live chat embeddings until the window elapsed, making one tier's back-pressure leak across the other. Give each tier its own EmbeddingProviderCooldown so a batch 429 only paces batch dispatch and a live 429 only paces live dispatch. Raise the live concurrency and rate defaults to match the now-independent live budget. Rewrite the cooldown tests to assert each tier succeeds while the other is in cooldown instead of asserting cross-tier blocking. - Construct a separate EmbeddingProviderCooldown per tier pacer - Raise live-max-concurrent-requests 4->8 and live-requests-per-second 3.0->8.0 - Assert per-tier cooldown isolation in OpenAiCompatibleEmbeddingClientTest
The default prefetch-limit and rag search-top-k pulled more candidates than the reranker and MMR stages use, spending Qdrant query budget on points that never reach the final answer. Lower the prefetch fan-out and the top-k cap to the figures the downstream stages actually consume. These are non-secret retrieval defaults; env-var overrides remain. - Lower HYBRID_PREFETCH_LIMIT default 20 -> 14 - Lower RAG_TOP_K default 12 -> 8
…ttom The scroll anchor disabled auto-scroll entirely once streaming began, so after a user clicked the new-content indicator to jump to the newest message, each subsequent streamed chunk rendered off-screen again and they had to keep clicking. The indicator's own copy promised it re-enabled following, but the composable never honored that. Introduce a jump-follows-stream mode: an explicit jumpToBottom arms followsActiveStreamAfterJump, and onContentAdded then keeps the view pinned to the bottom for every later chunk until a genuine user scroll away clears it. Add bounded post-scroll reconciliation passes so a late citation panel or highlighted code block that renders after the answer text cannot leave the final message tail hidden behind the composer. Update the indicator comment to match the new follow-until-scroll-away behavior. - Add followsActiveStreamAfterJump + activeStreamFollowScrollPending state - Pin to bottom in onContentAdded after an explicit jump until scroll-away - Bound performScroll with MAX_FINAL_REVEAL_RECONCILIATION_PASSES - Extract hideIndicatorInternal to hide without invalidating measurement - Add 34-case createScrollAnchor unit suite covering jump-follow and reconciliation
…API source Historical Java API mirrors hold near-duplicate API pages and dominate the broad official documentation corpus, so a generic request scoped to every official doc set retrieved redundant pages across old releases before the current one and diluted the reranker with stale copies. Narrow a broad official-doc request to the newest Java API source while retaining every non-Java documentation set, unless the request names an explicit release, carries a caller-owned release filter, or needs exact overload evidence -- those keep the full source scope for historical evidence. Add RetrievalConstraint.withDocSetScope to replace the documentation-set alternatives without intersecting, since this is a replacement, not a refinement of an existing scope. - Add RetrievalConstraint.withDocSetScope for non-intersecting scope replacement - Scope broad official requests to CURRENT_JAVA_API_DOCUMENTATION_DOC_SET in RetrievalService - Preserve full scope for versioned, caller-filtered, and exact-overload requests - Update RetrievalServiceTest to assert the current-Java-API scoped constraint
Bulk ingestion upserts used the fire-and-forget upsertAsync overload (wait=false), letting the pipeline queue writes faster than the shared Qdrant Cloud cluster applies them. During the java25-complete ingestion, search latency on the same cluster exceeded the 10s query deadline and dev retrieval failed with HybridSearchPartialFailureException (500s). Switch doUpsert to UpsertPoints with setWait(true), matching the contract replaceUrlDocuments already documents: applied writes pace bulk ingestion against cluster capacity, and the read-after-write point-identity verification observes the applied state. - Build UpsertPoints with setWait(true) in HybridVectorService.doUpsert - Move HybridVectorServiceTest stubs/verifications to the UpsertPoints overload and assert collection name plus wait=true on retried attempts
…robe The per-tier Retry-After cooldown rejects requests before any provider contact, but the warm-up probe treated that rejection as a provider failure, driving the health indicator DOWN and logging probe failures while the provider was never asked anything. Distinguish the locally recorded cooldown with EmbeddingProviderCooldownRejectionException, translate it in warmUp to EmbeddingProbeDeferredException, and log the actual deferral reason so a deferred probe keeps the last completed health observation unchanged. - Subtype EmbeddingServiceTemporarilyUnavailableException for the pre-contact cooldown rejection and throw it from the admission gate - Defer warmUp probes during a batch-tier cooldown without contacting the provider; genuine provider failures still record probe failures - Cover deferred-between-success-and-failure health accounting and probe deferral versus provider-failure paths in tests
A waiter coalesced onto an in-flight rerank inherited the owning caller's failure even when the owner died on its own tighter stage deadline: the waiter's remaining budget went unused and the request failed with a timeout it never caused. When the coalesced attempt fails with a deadline timeout and the waiter still owns stage budget, the waiter evicts the dead future and retries as the result owner. Non-timeout failures (empty or unparsable rerank responses, provider defects) keep propagating unchanged so permanent errors are never retried. - Wrap coalesced waits in a retry loop guarded by remaining budget and a TimeoutException cause-chain check in RerankerService - Cover owner-timeout recovery with exactly one cached result and non-timeout inheritance without a second dispatch in tests
The citation query and scroll assertions used partial containment, so dropping a metadata field from the include list would keep passing while production lost the field. Assert the selector equals the full metadata field set plus the content field so a dropped field fails loudly.
Assign immutable caching only to content-addressed assets and bounded public caching to unversioned fonts and the site manifest. Keep the HTML shell uncached, remove eager CSRF materialization from every static response, and verify that the explicit CSRF endpoint remains the sole browser token issuer.
…view Switching views through the header let a late streamed chunk land in the click-to-unmount gap, mutating chat state for a view the user already left. Route view selection through a guard that aborts the active stream before the chat view unmounts, exporting the cancellation from ChatView so the parent owns the handoff. - Export cancelActiveChatStream from ChatView and reuse it for unmount and new-message cancellation - Bind the ChatView instance in App and cancel on chat-to-other view transitions, including header clicks - Cover the click-to-unmount gap with a late-chunk regression test
…ize in WebKit Safari derives automatic optical sizing from the font size in points while Chromium uses CSS pixels, so Fraunces text below ~26px rendered with the font's small-text slab-serif design in Safari, most visibly a mismatched chunky "J" in the "Java Chat" brand text and headings. Pin the opsz axis to each rule's px font size so WebKit matches Chromium's rendering intent. - pin opsz 19 on the header brand text - pin opsz 23 on guided lesson titles - pin opsz 16-28 on assistant markdown headings by level - pin opsz 19/24 on lesson content h2/h3
Safari renders the "Java Chat" brand with Fraunces' small-optical-size J design, whose stub-hook letterform reads as a broken key instead of a J, while Georgia and Times New Roman render a normal hooked J. Safari computes automatic optical sizing in points rather than CSS pixels, and the vendored font's STAT table blocks any CSS-driven correction (font-variation-settings is ignored for this font in Safari; see google/fonts#7381). Use Times New Roman for the brand text so the J renders correctly in every browser without changing stroke weight or letter shape across environments.
The brand text read as too slight at 19px after the switch from Fraunces. Step it up to the next type-scale token at the same regular weight, as approved from the local preview.
Contributor
Important Review skippedToo many files! This PR contains 239 files, which is 139 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (269)
You can disable this status message by setting the 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:0eed5be19e
ℹ️ 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".
Comment on lines
+4
to
+5
| CANONICAL_MODEL_PATH = "src/main/java/com/williamcallahan/javachat/config/ModelConfiguration.java" | ||
| EXPECTED_CHAT_MODEL = "gpt-5.4" |
There was a problem hiding this comment.
Remove this scanner and its companion ast-grep rules: the canonical path, exact allowed model, provider markers, exclusions, and CI checks encode the repository's model policy in a second enforcement contract rather than leaving policy solely in AGENTS.md, which explicitly prohibits retaining policy mirrors and contracts.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
Comment on lines
+80
to
+84
| registry.addResourceHandler(CONTENT_HASHED_ASSET_PATH_PATTERN) | ||
| .addResourceLocations(CONTENT_HASHED_ASSET_LOCATION) | ||
| .setCacheControl(CacheControl.maxAge(CONTENT_HASHED_ASSET_CACHE_MAX_AGE) | ||
| .cachePublic() | ||
| .immutable()); |
There was a problem hiding this comment.
Restrict the year-long immutable policy to genuinely content-addressed files. The /assets/** directory also contains the stable public URL /assets/javachat_cup_star_256.png, referenced directly by Header.svelte and copied unchanged by Vite, so after that image is updated, any browser that cached the previous deployment can continue showing the old logo for a year because immutable responses are not revalidated.
Useful? React with 👍 / 👎.
Comment on lines
+44
to
+45
| if (currentView === 'chat' && selectedView !== 'chat') { | ||
| chatView?.cancelActiveChatStream() |
There was a problem hiding this comment.
When a user switches to Learn during an active chat response, this abort is handled by consumeSseStream as a normal return, while cancelActiveChatStream only clears the controller. Because the transcript now lives in module state, returning to Chat leaves either a user turn with no assistant answer or an unlabeled partial answer, and neither state exposes the new Retry action. Mark or remove the interrupted assistant turn before unmounting so the persisted conversation is not silently incomplete.
Useful? React with 👍 / 👎.
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
1 participant
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
Ships 141 commits that make the retrieval pipeline deadline-bounded end-to-end (one caller budget shared across search, Qdrant, rerank, and embedding hops), stream live retrieval progress to the browser during response preparation, and harden the chat UI against dropped streams, lost scroll position, and mobile keyboard/drawer glitches. Also lands Java 25 specification ingestion, embedding provider cooldown isolation, and ingestion failure-safety so a single bad document no longer aborts or corrupts a run.
Changes
Features
SseSupport,ChatController,RetrievalService)/learn/<slug>, and interrupted guided streams can be retried without losing the lesson context (LearnView, lesson routing in web layer)LocalDocsDirectoryIngestionService, MuPDF fetch contract)IngestionBacklogStatus,LocalIngestionRunStore)Bug Fixes
HybridSearchService,RerankerService,OpenAiCompatibleEmbeddingClient,SseSupport)RerankerService)OpenAiCompatibleEmbeddingClient,OpenAiProviderRoutingService)createScrollAnchor.svelte.ts)numericListFenceNesting.ts,MarkdownBlockContext)ChatInput,MobileChatDrawer.svelte)LocalDocsDirectoryIngestionService, ingestion scripts)OpenAIStreamingService,OpenAICompletionStatusTest)RetrievalService)Performance
HybridSearchService)MarkdownBlockContext)Refactoring
HybridSearchService)numericListFenceNesting.ts)Documentation
docs/retrieval-pipeline.md, API docs)scripts/fetch_all_docs.sh)Breaking Changes
None
Test Plan
HybridSearchServiceTest,RerankerServiceTest,OpenAIStreamingServiceTest,OpenAICompletionStatusTest,RetrievalServiceTest,SseSupportTest,createScrollAnchor.svelte.test.ts,ChatInput.test.ts,LearnView.test.ts, and ingestion failure-contract script testsmake buildandmake testpass on dev