feat(platform): complete guided learning and harden streaming, ingestion, and deploys - #109
Merged
Conversation
Pin the current Liberica release across mise, CI, Docker, and Gradle. Document the shared vendor and patch-version workflow for local development.
Bind the system prompt to AppProperties instead of a duplicate value injection. Keep marker clauses in the canonical prompt without format-string warnings.
Resolve supported reasoning values from the OpenAI SDK during bean creation. Fail lazy application startup immediately for invalid configuration.
Abort the active request when the chat view unmounts or is replaced. Ignore callbacks and failures from streams that no longer own the view state.
Ignore query strings and fragments when identifying PDF citations. Support case-insensitive HTTP schemes and local citation paths consistently.
Discard Unicode-blank enrichment markers without exposing raw source text. Keep CommonMark fences and their code indentation inside numbered list items.
Skip non-terminal title and prose abbreviations while finding trailing text. Treat corporate suffix punctuation as a valid sentence boundary.
Request strict JSON objects and reject preambles, malformed data, and unknown fields. Include context snippets in the default Spring cache identity.
Replace independently mutable health fields with one atomic snapshot. Report active checks accurately and keep failure timestamps monotonic.
Reserve provider capacity atomically immediately before SDK dispatch. Surface configured-provider failures without cross-provider fallback and remove dead retry protocol state. Accumulate only SSE chunks that survive bounded backpressure.
Synchronize rate-limit deadline reads, replacement, and expiry clearing. Prove concurrent checks cannot erase a newly recorded provider deadline.
Require the model response to contain every source index exactly once. Reject prose, fences, unknown fields, duplicates, omissions, and invalid indices.
Use final citation URLs as identity for resolvable sources. Keep member and page fragments distinct while retaining opaque local identities.
Resolve wrapped gRPC statuses through the canonical API and typed timeouts. Retry deadline and unavailable failures while rejecting quota and invalid-input statuses.
Remove replacement-character decoding from local file reads. Surface malformed source files as observable ingestion failures before storage.
Use one Java API path discriminator for remote and local ingestion. Verify manifest-mapped Javadocs reach the structured extractor.
Remove the legacy message alias and require a meaningful latest field. Reject missing, null, blank, Unicode-blank, and legacy request bodies at the HTTP boundary.
Bound and neutralize untrusted slug text before placing it in exception details. Cover control-character injection at the web boundary.
Bind startup validation and availability to the configured provider so mismatched credentials fail fast. Remove runtime fallback notices while preserving selected-provider SSE transparency and aligned documentation.
Clear pending off-screen update indicators when chat or lesson context changes without disturbing same-lesson retries. Split catalog and lesson-content lifecycles out of LearnView so each state owner remains bounded and independently testable.
Constructing an unselected provider client allowed stale secondary settings to abort application startup. Initialize only the provider selected by configuration and cover both provider directions. - Ignore invalid GitHub Models settings when OpenAI is selected - Ignore invalid OpenAI settings when GitHub Models is selected
The lesson update reset was coupled to an unapproved component split and incomplete asynchronous lifecycle handling. Keep the legacy LearnView boundary while making stream identity, cancellation, and provider status explicit. - Reset off-screen indicators across clear and lesson changes - Reject stale same-lesson citation callbacks and cancel streams on unmount - Surface the selected provider in chat and guided-chat status
Keep cooldown classification and rate-limit reservation under one routing-owned boundary so streaming cannot dispatch through a newly active cooldown. Permanent authentication, authorization, and not-found failures remain non-retryable instead of becoming transient on the next request. - validate configured provider backoff at startup - reserve immediately before completion or text-stream dispatch - cover permanent failures and subscription-time cooldown races
Contributor
📝 WalkthroughWalkthroughThe PR standardizes the Liberica Java toolchain, enforces explicit single-provider routing, removes streaming fallback notices, adds provider-aware SSE handling and cancellation guards, and tightens JSON, markdown, citation, ingestion, and concurrency behavior. ChangesConfigured provider and toolchain
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatView
participant ChatController
participant OpenAIStreamingService
participant ConfiguredProvider
Client->>ChatView: Send message
ChatView->>ChatController: Start stream with AbortSignal
ChatController->>OpenAIStreamingService: Request configured-provider stream
OpenAIStreamingService->>ConfiguredProvider: Admit and stream request
ConfiguredProvider-->>ChatController: Provider, status, text, and citations
ChatController-->>ChatView: SSE events
ChatView-->>Client: Render active stream
Possibly related issues
Possibly related PRs
Suggested labels: 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 tightens “single configured provider” behavior across backend + frontend streaming chat, hardens structured-output parsing (enrichment/rerank), improves citation identity fidelity, and aligns local/CI/container Java builds on a pinned BellSoft Liberica release.
Changes:
- Removes silent multi-provider fallback paths and surfaces provider identity via a dedicated SSE
providerevent. - Adds strict request/response contracts (canonical
latestquery for chat; strict JSON-only enrichment/rerank parsing). - Improves streaming lifecycle isolation in the UI (abort/ignore stale callbacks) and strengthens Markdown/enrichment/citation handling.
Reviewed changes
Copilot reviewed 101 out of 102 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/williamcallahan/javachat/web/SseSupportTest.java | Adds backpressure overflow assertions for coalesced chunk buffering/accumulation. |
| src/test/java/com/williamcallahan/javachat/web/GuidedSseCitationEventTest.java | Updates mocks for StreamingResult API change (no notices flux). |
| src/test/java/com/williamcallahan/javachat/web/GuidedLearningControllerTest.java | Verifies slug sanitization in exception message (control char removal). |
| src/test/java/com/williamcallahan/javachat/web/GuidedLearningControllerStreamingFailureTest.java | Updates mocks for StreamingResult API change. |
| src/test/java/com/williamcallahan/javachat/web/ChatStreamRequestTest.java | Adds request-contract tests requiring latest and rejecting legacy/malformed bodies. |
| src/test/java/com/williamcallahan/javachat/web/ChatControllerStreamingFailureTest.java | Updates controller callsites for new ChatStreamRequest shape and StreamingResult change. |
| src/test/java/com/williamcallahan/javachat/support/RetrievalErrorClassifierTest.java | Adds retry classification tests for timeout + gRPC statuses. |
| src/test/java/com/williamcallahan/javachat/service/StreamingAttemptContextTest.java | Removes tests for deleted multi-provider streaming-attempt context. |
| src/test/java/com/williamcallahan/javachat/service/RetrievalServiceTest.java | Expands citation identity tests for anchored Javadoc and PDF page fragments. |
| src/test/java/com/williamcallahan/javachat/service/RerankerServiceTest.java | Adds strict invalid JSON/order rejection tests; asserts deterministic ordering. |
| src/test/java/com/williamcallahan/javachat/service/RateLimitStateTest.java | Adds concurrent window-expiry and deadline-preservation tests. |
| src/test/java/com/williamcallahan/javachat/service/ProviderCircuitStateTest.java | Adds daily-limit reservation concurrency tests and injects deterministic time. |
| src/test/java/com/williamcallahan/javachat/service/OpenAiRequestFactoryTest.java | Adds startup validation tests for reasoning-effort config and normalization. |
| src/test/java/com/williamcallahan/javachat/service/markdown/MarkdownNormalizerTest.java | Adds normalization tests preserving fence indentation under numeric headers. |
| src/test/java/com/williamcallahan/javachat/service/markdown/MarkdownFenceRenderingTest.java | Adds HTML rendering assertions for fence fidelity after normalization. |
| src/test/java/com/williamcallahan/javachat/service/markdown/InlineListParserTest.java | Adds abbreviation-aware trailing-text parsing tests. |
| src/test/java/com/williamcallahan/javachat/service/markdown/EnrichmentPlaceholderizerTest.java | Adds Unicode-blank enrichment discard coverage and multiline-inline-code cases. |
| src/test/java/com/williamcallahan/javachat/service/ingestion/MalformedUtf8LocalDocsIngestionTest.java | Proves malformed UTF-8 is rejected pre-chunking/indexing with observable failure. |
| src/test/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessorTest.java | Verifies manifest-mapped local Javadocs use structured extraction + correct URL mapping. |
| src/test/java/com/williamcallahan/javachat/service/ExternalServiceHealthTest.java | Adds backoff overflow capping and healthSnapshot message/state agreement checks. |
| src/test/java/com/williamcallahan/javachat/service/EnrichmentServiceTest.java | Verifies JSON-object-only enrichment completion boundary and strict parsing failures. |
| src/test/java/com/williamcallahan/javachat/service/EnrichmentServiceCacheTest.java | Ensures enrichment cache identity includes context snippets. |
| src/test/java/com/williamcallahan/javachat/service/ComprehensiveListFormattingTest.java | Adds regression test for abbreviation + trailing prose formatting. |
| src/test/java/com/williamcallahan/javachat/domain/markdown/MarkdownEnrichmentTest.java | Verifies Unicode-blank rejection across all permitted enrichment variants. |
| src/test/java/com/williamcallahan/javachat/config/SystemPromptConfigTest.java | Aligns tests with AppProperties-owned docs JDK version binding. |
| src/test/java/com/williamcallahan/javachat/config/RequiredCredentialValidationTest.java | Updates validation expectations to provider-selected credential checks. |
| src/main/resources/application.properties | Removes implicit OpenAI key fallback from GitHub Models compatibility client; updates docs/comments. |
| src/main/resources/application-dev.properties | Mirrors provider-selection/no-fallback guidance in dev profile docs. |
| src/main/java/com/williamcallahan/javachat/web/SseSupport.java | Moves accumulation after bounded buffering; removes streaming-notice projection; trims SSE payload contract fields. |
| src/main/java/com/williamcallahan/javachat/web/SseConstants.java | Removes provider-fallback status code constant now that fallback is removed. |
| src/main/java/com/williamcallahan/javachat/web/GuidedLearningController.java | Removes streaming notice events; sanitizes slug in exception message. |
| src/main/java/com/williamcallahan/javachat/web/ChatStreamRequest.java | Enforces canonical latest query and removes legacy message support. |
| src/main/java/com/williamcallahan/javachat/web/ChatController.java | Switches to canonical latest query; removes runtime streaming notice merge. |
| src/main/java/com/williamcallahan/javachat/service/StreamingResult.java | Simplifies StreamingResult to (textChunks, provider) only. |
| src/main/java/com/williamcallahan/javachat/service/StreamingNoticeOrigin.java | Deletes notice-origin model no longer needed without fallback. |
| src/main/java/com/williamcallahan/javachat/service/StreamingNotice.java | Deletes streaming-notice model no longer needed without fallback. |
| src/main/java/com/williamcallahan/javachat/service/StreamingAttemptContext.java | Deletes provider-attempt context no longer needed without fallback. |
| src/main/java/com/williamcallahan/javachat/service/RetrievalService.java | Updates citation identity to preserve final anchors for resolvable sources. |
| src/main/java/com/williamcallahan/javachat/service/RerankerService.java | Enforces strict JSON-only, permutation-only rerank ordering; forbids coercions and trailing tokens. |
| src/main/java/com/williamcallahan/javachat/service/RateLimitState.java | Makes rate-limit window evaluation atomic and concurrency-safe. |
| src/main/java/com/williamcallahan/javachat/service/RateLimitService.java | Adds request reservation API; uses circuit+daily admission checks consistently. |
| src/main/java/com/williamcallahan/javachat/service/ProviderCircuitState.java | Adds daily-window reservation logic with deterministic time injection and atomic reset. |
| src/main/java/com/williamcallahan/javachat/service/OpenAiRequestFactory.java | Adds eager startup validation + strict reasoning-effort parsing (known values only). |
| src/main/java/com/williamcallahan/javachat/service/OpenAiProviderCandidate.java | Updates docs to reflect non-fallback semantics. |
| src/main/java/com/williamcallahan/javachat/service/markdown/MarkdownNormalizer.java | Improves list/fence indentation rules with CommonMark fence indentation limits. |
| src/main/java/com/williamcallahan/javachat/service/markdown/InlineListParser.java | Prevents abbreviation periods from prematurely terminating last list items. |
| src/main/java/com/williamcallahan/javachat/service/markdown/EnrichmentPlaceholderizer.java | Discards Unicode-blank enrichment markers and improves fence/inline-code tracking. |
| src/main/java/com/williamcallahan/javachat/service/markdown/CodeFenceStateTracker.java | Refactors to explicit fence transitions and backtick-run inline-code tracking. |
| src/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.java | Uses shared Java API URL classifier for local ingestion structured extraction selection. |
| src/main/java/com/williamcallahan/javachat/service/ingestion/JavaPackageExtractor.java | Centralizes Java API URL classification and reuses it for package extraction. |
| src/main/java/com/williamcallahan/javachat/service/FileOperationsService.java | Removes replacement-decoding fallback and enforces UTF-8 strict reads. |
| src/main/java/com/williamcallahan/javachat/service/EnrichmentService.java | Switches to JSON-object completion boundary and strict JSON parsing + safer cache keying. |
| src/main/java/com/williamcallahan/javachat/service/DocsIngestionService.java | Uses shared Java API classifier for remote ingestion extraction choice. |
| src/main/java/com/williamcallahan/javachat/service/ConfiguredProviderTemporarilyUnavailableException.java | Adds typed exception for “configured provider temporarily unavailable” admission failures. |
| src/main/java/com/williamcallahan/javachat/domain/markdown/Warning.java | Uses Unicode-blank-aware blank detection for enrichment validation. |
| src/main/java/com/williamcallahan/javachat/domain/markdown/Reminder.java | Uses Unicode-blank-aware blank detection for enrichment validation. |
| src/main/java/com/williamcallahan/javachat/domain/markdown/MarkdownEnrichment.java | Introduces Unicode-blank-aware blank detection helper. |
| src/main/java/com/williamcallahan/javachat/domain/markdown/Hint.java | Uses Unicode-blank-aware blank detection for enrichment validation. |
| src/main/java/com/williamcallahan/javachat/domain/markdown/Example.java | Uses Unicode-blank-aware blank detection for enrichment validation. |
| src/main/java/com/williamcallahan/javachat/domain/markdown/Background.java | Uses Unicode-blank-aware blank detection for enrichment validation. |
| src/main/java/com/williamcallahan/javachat/config/SystemPromptConfig.java | Binds prompt JDK version to AppProperties and uses placeholder replacement. |
| src/main/java/com/williamcallahan/javachat/config/RequiredCredentialValidation.java | Validates provider-selected credential availability at startup. |
| src/main/java/com/williamcallahan/javachat/config/ApiKeyLoggingConfig.java | Logs startup diagnostics based on configured provider selection (no fallback language). |
| README.md | Updates toolchain/vendor guidance and provider selection instructions. |
| frontend/src/lib/validation/schemas.ts | Updates SSE event schema to remove fallback fields and support provider event. |
| frontend/src/lib/utils/url.ts | Fixes PDF classification/labels when query/fragment present; safer display filename decoding. |
| frontend/src/lib/utils/url.test.ts | Adds PDF query/fragment/encoding coverage and hostname normalization checks. |
| frontend/src/lib/services/sse.test.ts | Updates status payload expectations; adds provider event dispatch coverage. |
| frontend/src/lib/services/markdown.ts | Avoids rendering enrichment cards for empty content; returns escaped text on parse failures. |
| frontend/src/lib/services/markdown.test.ts | Adds Unicode-blank enrichment rendering regression coverage. |
| frontend/src/lib/services/guided.ts | Forwards provider event callback through guided streaming. |
| frontend/src/lib/services/guided.test.ts | Updates terminal failure test; adds provider event forwarding coverage. |
| frontend/src/lib/services/chat.ts | Forwards provider event callback through chat streaming. |
| frontend/src/lib/services/chat.test.ts | Adds provider event forwarding coverage. |
| frontend/src/lib/composables/createStreamingState.svelte.ts | Tracks selected provider separately and composes status details consistently. |
| frontend/src/lib/components/MessageBubble.test.ts | Prevents Unicode-blank enrichment from appearing as fallback literal marker text. |
| frontend/src/lib/components/LearnView.svelte | Cancels/guards in-flight streams on unmount and lesson changes; captures provider event. |
| frontend/src/lib/components/CitationPanel.test.ts | Ensures decoded PDF labels don’t change href/anchors. |
| frontend/src/lib/components/ChatView.test.ts | Adds abort/unmount and stale-callback isolation tests; adds provider display checks. |
| frontend/src/lib/components/ChatView.svelte | Adds abort controllers + stale-callback guards; shows selected provider via shared streaming state. |
| frontend/src/lib/components/AssistantMarkdownBody.svelte | Removes raw-text fallback branch in favor of consistent rendered HTML path. |
| docs/retrieval-pipeline.md | Updates reranker parsing documentation to strict JSON/permutation requirements. |
| docs/pipeline-commands.md | Updates doc-set filtering and Java API seed/manifest guidance; clarifies Qdrant init behavior. |
| docs/getting-started.md | Updates setup to require selecting one provider + matching credential. |
| docs/configuration.md | Updates provider selection/no-fallback contract and reasoning-effort validation guidance. |
| docs/api.md | Documents new provider SSE event type. |
| Dockerfile | Switches backend build/runtime images to pinned Liberica. |
| CONTRIBUTING.md | Updates deterministic toolchain guidance to Liberica and pinning approach. |
| build.gradle.kts | Pins toolchain vendor to BellSoft. |
| .tool-versions | Pins Java tool version to Liberica 25.0.3+11. |
| .github/workflows/build.yml | Uses Liberica distribution + .tool-versions pin in CI. |
| .env.example | Updates to “select one provider” configuration and removes fallback language. |
| .config/lefthook.yml | Prevents oxfmt from formatting Svelte files; limits formatting to JS/TS. |
💡 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:7891408331
ℹ️ 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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
src/test/java/com/williamcallahan/javachat/service/markdown/InlineListParserTest.java (1)
182-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the newly introduced generic collection names with domain-specific roles.
src/test/java/com/williamcallahan/javachat/service/markdown/InlineListParserTest.java#L182-L196: renamelastItemTextstolastListLabels.src/test/java/com/williamcallahan/javachat/service/RerankerServiceTest.java#L75-L86: renameinvalidOrderingJsonValuestomalformedRerankJsonCases.As per coding guidelines, generic nouns such as
itemandvalue, including close variants, are prohibited.🤖 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/test/java/com/williamcallahan/javachat/service/markdown/InlineListParserTest.java` around lines 182 - 196, The collection in InlineListParserTest.java lines 182-196 should use the domain-specific name lastListLabels instead of lastItemTexts. In RerankerServiceTest.java lines 75-86, rename invalidOrderingJsonValues to malformedRerankJsonCases; update all references while preserving the existing test behavior.Source: Coding guidelines
src/main/java/com/williamcallahan/javachat/web/ChatController.java (1)
180-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLet
SseSupportown the event-ordering recipe.Both controllers now independently encode the same provider → status → data/heartbeat → citation protocol. A shared composition method would keep future event additions from becoming an exciting game of “spot the drift.”
src/main/java/com/williamcallahan/javachat/web/ChatController.java#L180-L185: replace the localFlux.concatsequence with the shared protocol method.src/main/java/com/williamcallahan/javachat/web/GuidedLearningController.java#L302-L307: use that same method for guided streams.As per coding guidelines, governed behavior must have one semantic owner, and duplicated logic should be reused.
🤖 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/web/ChatController.java` around lines 180 - 185, Move the shared provider → status → data/heartbeat → citation composition into an appropriate method on SseSupport, then replace the local Flux.concat recipes in ChatController and GuidedLearningController with calls to that method. Update src/main/java/com/williamcallahan/javachat/web/ChatController.java:180-185 and src/main/java/com/williamcallahan/javachat/web/GuidedLearningController.java:302-307; preserve the existing event ordering and stream behavior.Source: Coding guidelines
src/test/java/com/williamcallahan/javachat/service/OpenAIStreamingServiceTest.java (1)
52-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the configured-provider backoff once.
These
600literals encode one test policy. A named constant prevents the scenarios from quietly drifting apart.♻️ Proposed cleanup
class OpenAIStreamingServiceTest { + private static final long CONFIGURED_PROVIDER_BACKOFF_SECONDS = 600;- new OpenAiProviderRoutingService(rateLimitService, 600, configuredProvider.getName());+ new OpenAiProviderRoutingService(+ rateLimitService, CONFIGURED_PROVIDER_BACKOFF_SECONDS, configuredProvider.getName());Apply the same replacement to the remaining routing-service constructions.
As per coding guidelines, “No inline numbers (except 0, 1, -1) or strings; define named constants.” <coding_guidelines>
Also applies to: 92-99, 265-274, 300-310, 372-382
🤖 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/test/java/com/williamcallahan/javachat/service/OpenAIStreamingServiceTest.java` around lines 52 - 63, Define a named constant for the configured-provider backoff value of 600 in OpenAIStreamingServiceTest, then replace every corresponding inline 600 in the test scenarios and remaining routing-service constructions. Use that constant consistently while preserving the existing backoff behavior.Source: Coding guidelines
src/main/java/com/williamcallahan/javachat/service/ConfiguredProviderTemporarilyUnavailableException.java (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant null check.
Fun tidbit: Since
messageFor()is evaluated first as part of thesuper()call, it already checks and guarantees thatprovideris not null! You can safely skip the secondrequireNonNullcheck in the constructor body to keep things slightly cleaner. 🚀✨ Proposed refactor
ConfiguredProviderTemporarilyUnavailableException(RateLimitService.ApiProvider provider) { super(messageFor(provider)); - this.provider = Objects.requireNonNull(provider, "provider");+ this.provider = provider; }🤖 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/ConfiguredProviderTemporarilyUnavailableException.java` around lines 19 - 22, Remove the redundant Objects.requireNonNull call from the ConfiguredProviderTemporarilyUnavailableException constructor and assign provider directly after the super(messageFor(provider)) call, relying on messageFor’s existing null validation.
🤖 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 @.config/lefthook.yml:
- Line 10: Update the frontend format hook’s glob in .config/lefthook.yml to
include .svelte files alongside JavaScript and TypeScript files, ensuring staged
Svelte files are processed by the existing formatting command.
In `@frontend/src/lib/services/markdown.ts`:
- Around line 389-400: Update the diagnostic warning in the hasFences &&
!isBalanced branch to avoid logging enrichment or raw token contents. Retain the
kind field, and replace content and raw with bounded metadata such as their
lengths, keeping the existing development-only warning behavior unchanged.
In `@src/main/java/com/williamcallahan/javachat/service/EnrichmentService.java`:
- Around line 22-25: Move ENRICHMENT_TEMPERATURE and
ENRICHMENT_OUTPUT_TOKEN_BUDGET out of EnrichmentService and into AppProperties
as bound configuration fields, with defaults defined in the appropriate Spring
property files. Inject AppProperties into EnrichmentService and use its values
when configuring enrichment generation; do not add environment-variable handling
or retain hard-coded LLM settings.
In
`@src/main/java/com/williamcallahan/javachat/service/ExternalServiceHealth.java`:
- Around line 341-372: The health-check state flow around tryStartCheck,
markHealthy, markUnhealthy, and reset must reject completions from obsolete
checks. Add a generation/token to ServiceHealthState, have tryStartCheck return
or record the token for the started check, and require markHealthy/markUnhealthy
to update via CAS only when that token still matches the active check; ensure
reset advances the generation so in-flight callbacks cannot clear or overwrite
the newer check.
In
`@src/main/java/com/williamcallahan/javachat/service/ingestion/JavaPackageExtractor.java`:
- Around line 27-29: Update JavaPackageExtractor.isJavaApiUrl and extractPackage
to parse the URL and match JAVA_API_PATH_SEGMENT only against the parsed path,
excluding query and fragment content. Reuse the parsed-path result where
appropriate, preserve existing package extraction behavior for valid API URLs,
and add regression coverage for query or fragment values containing “/api/”.
In
`@src/main/java/com/williamcallahan/javachat/service/markdown/MarkdownNormalizer.java`:
- Around line 105-107: Update the first-pass normalization logic in
MarkdownNormalizer, around fenceTracker and
scanFenceMarkerAtCommonMarkIndentation, to recognize opening and closing fences
with zero to three leading spaces before rewriting lines. Ensure indented fenced
content remains untouched throughout the first pass, while preserving the
existing second-pass scanner behavior.
In
`@src/main/java/com/williamcallahan/javachat/service/ProviderCircuitState.java`:
- Around line 60-65: Update ProviderCircuitState.recordSuccess() so a late
success cannot clear a circuit opened by a newer rate-limit outcome. Preserve
circuitOpen while the current nextRetryTime deadline has not elapsed, or use
reservation/version tracking so only the latest outcome may close the circuit;
retain reset behavior once the circuit is no longer within its retry window.
In
`@src/main/java/com/williamcallahan/javachat/support/RetrievalErrorClassifier.java`:
- Around line 64-80: The classify method in RetrievalErrorClassifier must
recognize HTTP 503/service-unavailable failures as a retryable category instead
of UNKNOWN. Add a 503 or service-unavailable matching branch that returns the
existing retryable category used by the retry path, and add a regression test
confirming Qdrant HTTP 503 failures are classified accordingly.
In `@src/main/resources/application.properties`:
- Around line 34-36: Update the GitHub Models compatibility properties so
spring.ai.openai.api-key and spring.ai.openai.chat.api-key are sourced directly
from the process environment without blank defaults, and configure the GitHub
Models base URL from its environment variable. Keep this client restricted to
the GitHub Models provider rather than allowing an alternate provider URL.
In
`@src/test/java/com/williamcallahan/javachat/service/EnrichmentServiceTest.java`:
- Around line 33-38: Update EnrichmentServiceTest.setUp to use the
Spring-managed ObjectMapper from the focused test configuration established by
EnrichmentServiceCacheTest instead of manually constructing a new ObjectMapper.
Inject or autowire that mapper and pass it to EnrichmentService while preserving
the existing OpenAIStreamingService mock setup.
In
`@src/test/java/com/williamcallahan/javachat/service/OpenAiRequestFactoryTest.java`:
- Around line 33-46: Update invalidReasoningEffortFailsLazyApplicationStartup to
configure the reasoning-effort value through the canonical custom app.* Spring
property bound by AppProperties, rather than the prohibited
OPENAI_REASONING_EFFORT command-line key. Ensure the default is defined in the
appropriate Spring property files and start the test with the canonical property
key while preserving the existing invalid-value assertion.
---
Nitpick comments:
In
`@src/main/java/com/williamcallahan/javachat/service/ConfiguredProviderTemporarilyUnavailableException.java`:
- Around line 19-22: Remove the redundant Objects.requireNonNull call from the
ConfiguredProviderTemporarilyUnavailableException constructor and assign
provider directly after the super(messageFor(provider)) call, relying on
messageFor’s existing null validation.
In `@src/main/java/com/williamcallahan/javachat/web/ChatController.java`:
- Around line 180-185: Move the shared provider → status → data/heartbeat →
citation composition into an appropriate method on SseSupport, then replace the
local Flux.concat recipes in ChatController and GuidedLearningController with
calls to that method. Update
src/main/java/com/williamcallahan/javachat/web/ChatController.java:180-185 and
src/main/java/com/williamcallahan/javachat/web/GuidedLearningController.java:302-307;
preserve the existing event ordering and stream behavior.
In
`@src/test/java/com/williamcallahan/javachat/service/markdown/InlineListParserTest.java`:
- Around line 182-196: The collection in InlineListParserTest.java lines 182-196
should use the domain-specific name lastListLabels instead of lastItemTexts. In
RerankerServiceTest.java lines 75-86, rename invalidOrderingJsonValues to
malformedRerankJsonCases; update all references while preserving the existing
test behavior.
In
`@src/test/java/com/williamcallahan/javachat/service/OpenAIStreamingServiceTest.java`:
- Around line 52-63: Define a named constant for the configured-provider backoff
value of 600 in OpenAIStreamingServiceTest, then replace every corresponding
inline 600 in the test scenarios and remaining routing-service constructions.
Use that constant consistently while preserving the existing backoff behavior.
🪄 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: 1bf281e4-65b9-48f4-a5ee-74575e85114e
📒 Files selected for processing (102)
.config/lefthook.yml.env.example.github/workflows/build.yml.tool-versionsCONTRIBUTING.mdDockerfileREADME.mdbuild.gradle.ktsdocs/api.mddocs/configuration.mddocs/development.mddocs/getting-started.mddocs/pipeline-commands.mddocs/retrieval-pipeline.mdfrontend/src/lib/components/AssistantMarkdownBody.sveltefrontend/src/lib/components/ChatView.sveltefrontend/src/lib/components/ChatView.test.tsfrontend/src/lib/components/CitationPanel.test.tsfrontend/src/lib/components/LearnView.sveltefrontend/src/lib/components/LearnView.test.tsfrontend/src/lib/components/MessageBubble.test.tsfrontend/src/lib/composables/createStreamingState.svelte.tsfrontend/src/lib/services/chat.test.tsfrontend/src/lib/services/chat.tsfrontend/src/lib/services/guided.test.tsfrontend/src/lib/services/guided.tsfrontend/src/lib/services/markdown.test.tsfrontend/src/lib/services/markdown.tsfrontend/src/lib/services/sse.test.tsfrontend/src/lib/utils/url.test.tsfrontend/src/lib/utils/url.tsfrontend/src/lib/validation/schemas.tssrc/main/java/com/williamcallahan/javachat/config/ApiKeyLoggingConfig.javasrc/main/java/com/williamcallahan/javachat/config/RequiredCredentialValidation.javasrc/main/java/com/williamcallahan/javachat/config/SystemPromptConfig.javasrc/main/java/com/williamcallahan/javachat/domain/markdown/Background.javasrc/main/java/com/williamcallahan/javachat/domain/markdown/Example.javasrc/main/java/com/williamcallahan/javachat/domain/markdown/Hint.javasrc/main/java/com/williamcallahan/javachat/domain/markdown/MarkdownEnrichment.javasrc/main/java/com/williamcallahan/javachat/domain/markdown/Reminder.javasrc/main/java/com/williamcallahan/javachat/domain/markdown/Warning.javasrc/main/java/com/williamcallahan/javachat/service/ConfiguredProviderTemporarilyUnavailableException.javasrc/main/java/com/williamcallahan/javachat/service/DocsIngestionService.javasrc/main/java/com/williamcallahan/javachat/service/EnrichmentService.javasrc/main/java/com/williamcallahan/javachat/service/ExternalServiceHealth.javasrc/main/java/com/williamcallahan/javachat/service/FileOperationsService.javasrc/main/java/com/williamcallahan/javachat/service/OpenAIStreamingService.javasrc/main/java/com/williamcallahan/javachat/service/OpenAiProviderCandidate.javasrc/main/java/com/williamcallahan/javachat/service/OpenAiProviderRoutingService.javasrc/main/java/com/williamcallahan/javachat/service/OpenAiRequestFactory.javasrc/main/java/com/williamcallahan/javachat/service/ProviderCircuitState.javasrc/main/java/com/williamcallahan/javachat/service/RateLimitService.javasrc/main/java/com/williamcallahan/javachat/service/RateLimitState.javasrc/main/java/com/williamcallahan/javachat/service/RerankerService.javasrc/main/java/com/williamcallahan/javachat/service/RetrievalService.javasrc/main/java/com/williamcallahan/javachat/service/StreamingAttemptContext.javasrc/main/java/com/williamcallahan/javachat/service/StreamingNotice.javasrc/main/java/com/williamcallahan/javachat/service/StreamingNoticeOrigin.javasrc/main/java/com/williamcallahan/javachat/service/StreamingResult.javasrc/main/java/com/williamcallahan/javachat/service/ingestion/JavaPackageExtractor.javasrc/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.javasrc/main/java/com/williamcallahan/javachat/service/markdown/CodeFenceStateTracker.javasrc/main/java/com/williamcallahan/javachat/service/markdown/EnrichmentPlaceholderizer.javasrc/main/java/com/williamcallahan/javachat/service/markdown/InlineListParser.javasrc/main/java/com/williamcallahan/javachat/service/markdown/MarkdownNormalizer.javasrc/main/java/com/williamcallahan/javachat/support/RetrievalErrorClassifier.javasrc/main/java/com/williamcallahan/javachat/web/ChatController.javasrc/main/java/com/williamcallahan/javachat/web/ChatStreamRequest.javasrc/main/java/com/williamcallahan/javachat/web/GuidedLearningController.javasrc/main/java/com/williamcallahan/javachat/web/SseConstants.javasrc/main/java/com/williamcallahan/javachat/web/SseSupport.javasrc/main/resources/application-dev.propertiessrc/main/resources/application.propertiessrc/test/java/com/williamcallahan/javachat/config/RequiredCredentialValidationTest.javasrc/test/java/com/williamcallahan/javachat/config/SystemPromptConfigTest.javasrc/test/java/com/williamcallahan/javachat/domain/markdown/MarkdownEnrichmentTest.javasrc/test/java/com/williamcallahan/javachat/service/ComprehensiveListFormattingTest.javasrc/test/java/com/williamcallahan/javachat/service/EnrichmentServiceCacheTest.javasrc/test/java/com/williamcallahan/javachat/service/EnrichmentServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/ExternalServiceHealthTest.javasrc/test/java/com/williamcallahan/javachat/service/OpenAIStreamingServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/OpenAiProviderRoutingServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/OpenAiRequestFactoryTest.javasrc/test/java/com/williamcallahan/javachat/service/ProviderCircuitStateTest.javasrc/test/java/com/williamcallahan/javachat/service/RateLimitStateTest.javasrc/test/java/com/williamcallahan/javachat/service/RerankerServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/RetrievalServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/StreamingAttemptContextTest.javasrc/test/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessorTest.javasrc/test/java/com/williamcallahan/javachat/service/ingestion/MalformedUtf8LocalDocsIngestionTest.javasrc/test/java/com/williamcallahan/javachat/service/markdown/EnrichmentPlaceholderizerTest.javasrc/test/java/com/williamcallahan/javachat/service/markdown/InlineListParserTest.javasrc/test/java/com/williamcallahan/javachat/service/markdown/MarkdownFenceRenderingTest.javasrc/test/java/com/williamcallahan/javachat/service/markdown/MarkdownNormalizerTest.javasrc/test/java/com/williamcallahan/javachat/support/RetrievalErrorClassifierTest.javasrc/test/java/com/williamcallahan/javachat/web/ChatControllerStreamingFailureTest.javasrc/test/java/com/williamcallahan/javachat/web/ChatStreamRequestTest.javasrc/test/java/com/williamcallahan/javachat/web/GuidedLearningControllerStreamingFailureTest.javasrc/test/java/com/williamcallahan/javachat/web/GuidedLearningControllerTest.javasrc/test/java/com/williamcallahan/javachat/web/GuidedSseCitationEventTest.javasrc/test/java/com/williamcallahan/javachat/web/SseSupportTest.javasrc/test/java/com/williamcallahan/javachat/web/StreamingProviderFallbackSseEventTest.java
💤 Files with no reviewable changes (7)
- src/test/java/com/williamcallahan/javachat/service/StreamingAttemptContextTest.java
- src/main/java/com/williamcallahan/javachat/service/StreamingNoticeOrigin.java
- src/main/java/com/williamcallahan/javachat/web/SseConstants.java
- src/main/java/com/williamcallahan/javachat/service/StreamingNotice.java
- src/main/java/com/williamcallahan/javachat/service/StreamingAttemptContext.java
- src/test/java/com/williamcallahan/javachat/web/StreamingProviderFallbackSseEventTest.java
- frontend/src/lib/validation/schemas.ts
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.
…st-marker-detection-references-in-pa-6c87a0 docs: update list marker detection references in parsing docs
…ient-side-markdown-processing-for-sv-107ab3 docs: update client-side markdown processing for Svelte frontend
…ng-sse-provider-event-and-full-statu-65a6b2 docs: add missing SSE provider event and full status/error field shapes
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/domains/all-parsing-and-markdown-logic.md`:
- Around line 143-172: Replace the detailed “Svelte Frontend Markdown
Processing” implementation summary with a responsibility-level architecture
description and links to the canonical frontend documentation or source files.
Remove mirrored file paths, function names, parser-mode details, and rendering
behavior from this document, or generate the section from the canonical owners
so it cannot drift.
- Around line 281-283: Reconcile the Svelte architecture description with the
retained legacy references in the sections around markdown-utils.js,
/render/structured, and chat.html. Clearly label those paragraphs as
historical/legacy and identify the server-authoritative Svelte pipeline as
current, or remove obsolete details so the document presents one unambiguous
authoritative flow.
🪄 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: e37a4252-7810-4ac8-8d27-f9a349551d37
📒 Files selected for processing (3)
docs/api.mddocs/domains/all-parsing-and-markdown-logic.mdfrontend/src/lib/services/markdown.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/api.md
- frontend/src/lib/services/markdown.ts
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Documentation fetchers, Java catalog readers, and ingestion markers previously restated source identities and extraction behavior. The canonical manifest now drives every projection and semantic versioning forces stale documents to reindex. - Project Java and shell catalogs from one manifest - Version local ingestion markers by extractor semantics - Classify Java API sources by manifest-backed URI paths
Every table-of-contents lesson now has curated, packaged content, so navigation cannot land on a lesson whose source is missing at runtime. - Add nine complete lesson resources - Verify every lesson slug resolves to meaningful content - Exclude local curation scratch work from version control
Enrichment kinds could diverge across Java, frontend, prompts, and styles, while incomplete streams could misread code fences as marker boundaries. Every consumer now projects one manifest and both parsers honor CommonMark code regions. - Replace five mirrored domain variants with one manifest-backed record - Protect 0-3 space fences, indented code, and multiline backticks - Reject invisible-only markers and emit bounded development diagnostics
Replaced or overloaded streams could mutate current lesson state, and bounded overflow could expose partial text without a terminal contract. Request ownership now survives every await and all failed streams end consistently without persistence. - Abort stale citation and lesson work across replacement and teardown - Fail bounded SSE overflow with a retryable terminal error - Share configured-provider cooldown handling across chat modes
Generation knobs and embedding credentials had multiple owners, allowing startup behavior and scripts to disagree. Non-secret policy now binds through typed application properties while credentials enter only through approved environment variables. - Centralize chat, enrichment, and reranker generation settings - Bind Qdrant and embedding secrets directly from the environment - Remove unused Spring AI chat auto-configuration
Readiness alone could not distinguish a fresh deployment from a healthy stale container. The public sanitized build-info endpoint and Docker smoke now require both build identities to match the source commit. - Expose only actuator health and sanitized info publicly - Assert deployment and build commits in the container smoke job - Document the complete single-provider SSE contract
Merge the reviewed remote dev documentation commits without rewriting local history. Conflict resolution preserves the current single-provider SSE schema, strict reranker contract, and canonical Spring Framework selector while retaining the accepted README, CI, and parsing documentation corrections.
The focused cache context constructs AppProperties without Spring Boot property binding, so it must provide the same non-secret embedding settings that production receives from application.properties.
Serialize shared build outputs with a kernel-managed lock and keep quality gates strict. Eliminate JDK, Mockito, Oxlint, static-analysis, and expected negative-path log noise.
The non-secret Novita endpoint and model were removed from deployment ownership while the credential remained active, causing the new container to fail startup. Bind those settings from application properties, keep runtime and ingestion provider selection aligned, and cover the deployed configuration shape with startup tests.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 18, 2026
This was referenced Jul 21, 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.
Summary
Java learners can now open every guided lesson, receive stable streamed answers from the explicitly selected LLM provider, and navigate without obsolete lesson or chat callbacks changing the active view. Ingestion and model-output contracts now fail clearly on ambiguous input, while local, CI, and container verification share one reproducible toolchain and deployments prove the exact source revision they run.
Changes
Learner experience
GuidedLearningService,src/main/resources/guided/lessons/).ChatView,LearnView).SseSupport,ChatController,GuidedLearningController).MarkdownNormalizer,InlineListParser,RetrievalService,frontend/src/lib/utils/url.ts).Provider, model, and ingestion contracts
OpenAiProviderRoutingService,OpenAIStreamingService).latestinput; enrichment accepts one complete typed JSON object; reranking requires an exact document permutation (ChatStreamRequest,EnrichmentService,RerankerService).FileOperationsService,JavaPackageExtractor,IngestionProvenanceDeriver).documentation-sources.manifest,java-api-documentation-sources.manifest,enrichment-kinds.manifest).Configuration, verification, and deployment
AppProperties,RequiredCredentialValidation,QdrantClientConfig,RemoteEmbedding).application.properties; credentials activate the provider without restoring removed environment aliases, runtime and ingestion preflight apply the same selection semantics, and deployment-shaped startup tests cover both remote and explicit OpenAI selection (EmbeddingConfig,RemoteEmbedding,embedding_preflight.sh,EmbeddingConfigStartupTest)..tool-versions,build.gradle.kts,.github/workflows/build.yml,Dockerfile).BuildStateLock,Makefile,ExpectedLogEvents).SecurityConfig,JavaChatApplicationTests,.github/workflows/build.yml).Breaking changes
LLM_PRIMARY_PROVIDERtogithub_modelsoropenai, provide that provider's matching credential, and restart to switch providers.POST /api/chat/streamacceptslatestonly; clients using the legacymessagefield must migrate.providerevent; removed fallback attempt fields are no longer emitted.Validation
a214437: both the push run and pull-request run passed frontend, Java build, Ubuntu build-lock smoke, and exact-SHA Docker runtime identity checks.hpkpo3o35k7uj9wamqqizn2f:deployment.commitandbuild.commitboth equala21443783082f4beddd1f3539c5fe2ffa51fa2a9; health, readiness, liveness, and dependency health remained HTTP 200/UPacross two checks 30 seconds apart.dev.javachat.ai: cited chat streaming, guided lesson content, lesson-scoped chat, desktop/mobile navigation, and all observed API requests passed with no browser console or page errors.