fix(runtime): reliable Gemma routing, streaming recovery, embeddings, and ingestion - #71
Merged
Conversation
Make the shell pipeline the sole owner of collision-safe collection names and require its resolved name at the Java boundary. Run ingestion from an immutable staged JAR so concurrent builds cannot mutate the active archive. - Preserve canonical names for existing simple repository owners - Reject identity mismatches before synchronization - Stage and validate the executable application archive
Prune and force-reindex tracked files whenever their stored Qdrant point coverage is incomplete, even when the local fingerprint has not changed. - Verify stored point counts before skipping unchanged files - Rebuild missing vectors through the strict reindex path
Create every Qdrant future inside the retry callback so transient failures start a fresh upsert, delete, or count operation. - Reissue failed asynchronous calls on every retry - Cover all three operations with transient-failure tests
Synchronize circuit state transitions so availability checks cannot observe an open circuit before its retry deadline is published. - Initialize the retry deadline to a valid instant - Publish rate-limit state in one synchronized transition - Exercise concurrent opens and availability checks
Wait for the latest constrained rate-limit bucket and preserve provider-declared reset deadlines across repeated failures. Verify caller cancellation leaves an otherwise healthy primary provider eligible. - Select the longest positive reset window - Remove compounding application backoff from persistent state - Cover cancellation-aware provider routing
Return stable client-safe messages and diagnostics without exposing servlet messages, provider bodies, headers, filesystem paths, or exception text. - Limit diagnostics to exception type and numeric status - Derive public messages from trusted status values - Cover secret-bearing downstream and servlet failures
Reference-count the shared text and heartbeat stream so cancellation of both subscribers disconnects the upstream request. - Preserve the two-subscriber rendezvous - Verify client disposal cancels the shared upstream
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates GitHub ingestion identity handling, markdown enrichment parsing, application JAR staging, provider retries and timeouts, embedding probes, rate-limit state, ingestion recovery, web error sanitization, and SSE cancellation behavior, with expanded regression and concurrency tests. ChangesApplication behavior updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Stabilizes Java Chat’s LLM gateway usage (Gemma alias), strengthens pre-text streaming recovery, and hardens adjacent ingestion/embeddings and client-facing error-safety behaviors that surfaced during dogfooding.
Changes:
- Adds bounded pre-text streaming retry logic and related routing/circuit-state concurrency guarantees.
- Reworks embedding and Qdrant retry behavior to avoid duplicated retry loops and ensure retries re-create async operations.
- Tightens public error responses (no downstream details) and improves frontend enrichment-marker parsing resilience.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/williamcallahan/javachat/web/SseSupport.java | Shares SSE upstream via refCount(2) so upstream cancels when both consumers cancel. |
| src/test/java/com/williamcallahan/javachat/web/SseSupportTest.java | Adds coverage asserting upstream cancellation behavior with merged text/heartbeat streams. |
| src/main/java/com/williamcallahan/javachat/web/IngestionController.java | Removes downstream exception messages from ingestion API responses; standardizes validation failures. |
| src/test/java/com/williamcallahan/javachat/web/IngestionControllerTest.java | Verifies MVC rejects oversized ingestion requests early and omits downstream secrets. |
| src/main/java/com/williamcallahan/javachat/web/ExceptionResponseBuilder.java | Makes exception diagnostics client-safe (type + HTTP status only). |
| src/test/java/com/williamcallahan/javachat/web/ExceptionResponseBuilderTest.java | Ensures headers/bodies/provider fields are not leaked in exception diagnostics. |
| src/main/java/com/williamcallahan/javachat/web/CustomErrorController.java | Returns stable API error messages and avoids exposing servlet error text. |
| src/test/java/com/williamcallahan/javachat/web/CustomErrorControllerTest.java | Adds tests preventing servlet error-message exposure and handling non-Exception throwables. |
| src/main/java/com/williamcallahan/javachat/web/BaseController.java | Stops propagating exception messages in generic 500 responses. |
| src/main/java/com/williamcallahan/javachat/service/StreamingAttemptContext.java | Introduces bounded attempt model (single-provider retry once; multi-provider try each once). |
| src/test/java/com/williamcallahan/javachat/service/StreamingAttemptContextTest.java | Covers bounded attempt sequencing rules for single vs multi-provider routing. |
| src/main/java/com/williamcallahan/javachat/service/OpenAIStreamingService.java | Implements “retry before first text” semantics and threads caller-owned timeouts into requests. |
| src/test/java/com/williamcallahan/javachat/service/OpenAIStreamingServiceTest.java | Adds routing assertions ensuring caller cancellation doesn’t penalize provider eligibility. |
| src/main/java/com/williamcallahan/javachat/service/RerankerService.java | Raises reranker completion budget and passes timeout through to streaming service completion call. |
| src/test/java/com/williamcallahan/javachat/service/RerankerServiceTest.java | Verifies reranker uses configured budget and timeout wiring. |
| src/main/java/com/williamcallahan/javachat/service/RateLimitHeaderParser.java | Chooses latest positive reset bucket and simplifies reset-candidate selection. |
| src/test/java/com/williamcallahan/javachat/service/RateLimitHeaderParserTest.java | Adds coverage for choosing the latest positive reset window. |
| src/main/java/com/williamcallahan/javachat/service/RateLimitState.java | Records provider-declared reset times without additional exponential backoff. |
| src/test/java/com/williamcallahan/javachat/service/RateLimitStateTest.java | Ensures repeated failures preserve reset times and track consecutive failures. |
| src/main/java/com/williamcallahan/javachat/service/ProviderCircuitState.java | Synchronizes state transitions to avoid partial visibility during concurrent checks/updates. |
| src/test/java/com/williamcallahan/javachat/service/ProviderCircuitStateTest.java | Stress-tests concurrent transition + availability checks for atomicity. |
| src/main/java/com/williamcallahan/javachat/service/OpenAiCompatibleEmbeddingClient.java | Removes app-level embedding retries; defers probes during active foreground embedding; tier-specific SDK retry policy. |
| src/test/java/com/williamcallahan/javachat/service/OpenAiCompatibleEmbeddingClientTest.java | Updates retry expectations and adds concurrency/probe-deferral and SDK retry budget tests. |
| src/main/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAlive.java | Treats probe deferral as non-failure and preserves last completed health observation. |
| src/test/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAliveTest.java | Adds coverage for deferred probes preserving prior health details. |
| src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java | Recreates Qdrant async operations per retry to avoid re-awaiting a failed future. |
| src/test/java/com/williamcallahan/javachat/service/HybridVectorServiceTest.java | Verifies retried Qdrant operations start fresh async calls. |
| src/main/java/com/williamcallahan/javachat/service/ingestion/SourceCodeFileIngestionProcessor.java | Forces reindex when markers exist but point coverage is missing; prunes prior points before re-upsert. |
| src/test/java/com/williamcallahan/javachat/service/ingestion/SourceCodeFileIngestionProcessorTest.java | Adds coverage for “unchanged but missing points” forcing reindex and collection-name consistency. |
| src/main/java/com/williamcallahan/javachat/domain/ingestion/GitHubRepositoryIdentity.java | Removes Java-side canonical collection naming in favor of script-owned canonicalization. |
| src/test/java/com/williamcallahan/javachat/domain/ingestion/GitHubRepositoryIdentityTest.java | Updates identity tests to focus on canonical key/URL only. |
| src/test/java/com/williamcallahan/javachat/domain/ingestion/GitHubCollectionNamingScriptTest.java | Adds tests for canonical collection naming/validation implemented in shell script. |
| src/main/java/com/williamcallahan/javachat/cli/GitHubRepoProcessor.java | Requires explicit GITHUB_COLLECTION_NAME env var (no Java fallback naming). |
| scripts/lib/github_identity.sh | Defines canonical collection naming boundary and adds require_canonical_collection_name. |
| scripts/process_github_repo.sh | Uses canonical-name requirement helper and stages JAR to prevent mutation while running. |
| scripts/lib/common_qdrant.sh | Adds stage_app_jar helper to copy + lock down runnable JAR. |
| docs/github-repository-ingestion.md | Documents script as canonical owner for collection-name validation and index inventory. |
| frontend/src/lib/services/markdown.ts | Hardens enrichment-marker parsing to suppress malformed markers while preserving braces/code fences. |
| frontend/src/lib/services/markdown.test.ts | Adds regression tests for stray braces, truncated markers, nesting recovery, and fenced-code preservation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+65
| Instant parsedResetTime = parser.parseResetInstant(headers).orElseThrow(); | ||
| long remainingResetSeconds = | ||
| Duration.between(Instant.now(), parsedResetTime).getSeconds(); | ||
| assertTrue(remainingResetSeconds >= 58, "Expected the 60-second token reset window"); | ||
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6512b3929e
ℹ️ 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
+341
to
343
| if (!attemptContext.hasNextAttempt() | ||
| || emittedTextChunk.get() | ||
| || !providerRoutingService.isStreamingFallbackEligible(streamingFailure)) { |
There was a problem hiding this comment.
When only one provider is configured, the new attempt model allows a second attempt against the same provider. Since isStreamingFallbackEligible treats HTTP 429/rate-limit failures as retryable and executeStreamingRequest records the rate-limit before this branch, a stream that receives a Retry-After/429 before text immediately calls the same provider again instead of honoring the newly opened availability window. This spends an extra rate-limited request and can keep failing users during provider throttling; only retry same-provider failures that are not rate-limit windows, or reselect/check provider availability before retrying.
Useful? React with 👍 / 👎.
Comment on lines
255
to
257
| RequestOptions requestOptions = RequestOptions.builder() | ||
| .timeout(completeTimeout()) | ||
| .timeout(completeTimeout(configuration.requestTimeout())) | ||
| .build(); |
There was a problem hiding this comment.
This timeout is applied separately to each provider attempt inside the fallback loop. For callers like RerankerService that pass rerankerTimeout expecting the whole LLM rerank to be bounded (the previous code wrapped completeJsonObject(...).timeout(rerankerTimeout)), a primary request can consume the full timeout and then a fallback provider can consume another full timeout, so a 45s reranker budget can block for about 90s. Track a deadline/remaining duration across the loop or keep an outer Reactor timeout.
Useful? React with 👍 / 👎.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/com/williamcallahan/javachat/web/IngestionController.java (1)
62-73: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winServer logs no longer capture any exception detail either — full diagnosability is lost.
Now that the client response omits
exception.getMessage(), theselog.error(...)calls become the only remaining diagnostic trail — but they only logexception.getClass().getSimpleName(), never the exception itself. Without the message or stack trace, on-call engineers get no more insight from the logs than the sanitized client response gives.CustomErrorController.logRequestFailurein this same PR already does it right by passing the exception as the trailing SLF4J arg (auto-triggers stack trace logging) — worth mirroring that here.🔍 Proposed fix: log the exception, not just its class name
} catch (IOException ioException) { - log.error(- "IO error during ingestion (exception type: {})",- ioException.getClass().getSimpleName());+ log.error(+ "IO error during ingestion (exception type: {})",+ ioException.getClass().getSimpleName(),+ ioException); return buildIngestionError(HttpStatus.INTERNAL_SERVER_ERROR, "Failed to ingest documents", ioException); } catch (RuntimeException runtimeException) { - log.error(- "Unexpected error during ingestion (exception type: {})",- runtimeException.getClass().getSimpleName());+ log.error(+ "Unexpected error during ingestion (exception type: {})",+ runtimeException.getClass().getSimpleName(),+ runtimeException); return buildIngestionError( HttpStatus.INTERNAL_SERVER_ERROR, "Failed to perform ingestion", runtimeException); }Apply the same pattern to the
ingestLocalcatch blocks (IOException/RuntimeException).Also applies to: 88-100
🤖 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/IngestionController.java` around lines 62 - 73, Update the IOException and RuntimeException catch blocks in ingestLocal to pass the caught exception as the trailing argument to log.error, matching CustomErrorController.logRequestFailure and preserving the existing contextual messages.src/main/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAlive.java (1)
104-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTidbit: precompute the log-safe model name once.
modelNameisfinal, yet the\r/\nescaping is now recomputed in three places (recordDeferred,recordSuccess,recordFailure). Since it never changes, computing it once in the constructor keeps things DRY and shaves a little work off every probe.♻️ Proposed refactor
private final EmbeddingClient embeddingClient; private final String modelName; + private final String logSafeModelName; private final LongSupplier nanoTime;EmbeddingModelKeepAlive(EmbeddingClient embeddingClient, LongSupplier nanoTime) { this.embeddingClient = Objects.requireNonNull(embeddingClient, "embeddingClient"); this.modelName = Objects.requireNonNull(embeddingClient.modelName(), "embeddingClient.modelName"); + this.logSafeModelName = modelName.replace("\r", "\\r").replace("\n", "\\n"); this.nanoTime = Objects.requireNonNull(nanoTime, "nanoTime"); }private void recordDeferred(long probeDurationMillis) { - String logSafeModelName = modelName.replace("\r", "\\r").replace("\n", "\\n"); log.atDebug().log(() -> "event=embedding_model_probe_deferred outcome=deferred model=" + logSafeModelName + " durationMs=" + probeDurationMillis + " reason=foreground_embedding_active"); }Then drop the local re-derivation in
recordSuccess/recordFailuretoo.🤖 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/EmbeddingModelKeepAlive.java` around lines 104 - 108, Precompute the escaped model name once during construction of EmbeddingModelKeepAlive and store it in a field, using the final modelName value. Update recordDeferred, recordSuccess, and recordFailure to reuse that field and remove their local CR/LF escaping.frontend/src/lib/services/markdown.ts (1)
212-220: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor: precompute the opening prefixes once and short-circuit on
{.
readEnrichmentOpeningrebuilds`{{${kind}:}`strings and re-derivesObject.keys(ENRICHMENT_KINDS)on every call — and it's now called for every non-fenced character position insidefindEnrichmentClose's scan loop (line 281). A quicksrc[index] !== "{"guard plus a module-level precomputed prefix list would cut a lot of wasted work without changing behavior.♻️ Proposed refactor
+const ENRICHMENT_OPENING_PREFIXES = Object.keys(ENRICHMENT_KINDS).map((kind) => ({+ kind,+ prefix: `{{${kind}:`,+}));+ function readEnrichmentOpening(src: string, index: number): EnrichmentOpening | null { - for (const kind of Object.keys(ENRICHMENT_KINDS)) {- const opening = `{{${kind}:`;- if (src.startsWith(opening, index)) {- return { kind, length: opening.length };- }+ if (src[index] !== "{") {+ return null;+ }+ for (const { kind, prefix } of ENRICHMENT_OPENING_PREFIXES) {+ if (src.startsWith(prefix, index)) {+ return { kind, length: prefix.length };+ } } return null; }Also applies to: 281-284
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/lib/services/markdown.ts` around lines 212 - 220, Optimize readEnrichmentOpening by adding an early return when src[index] is not "{", and move the Object.keys(ENRICHMENT_KINDS)-derived opening prefixes into a module-level precomputed collection. Reuse those prefixes during matching instead of rebuilding strings and enumerating kinds on each call, preserving the existing EnrichmentOpening result behavior for valid openings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/lib/services/markdown.ts`:
- Around line 330-331: Rename the local variable content in the rewritten
function to a domain-specific name that reflects the extracted markdown segment,
and update all references to it within that function. Leave the raw extraction
logic unchanged.
In `@scripts/lib/github_identity.sh`:
- Around line 115-120: Update the segment encoding logic used by the canonical
collection name flow in github_identity.sh, including the transformed-name
handling around encoded_owner_segment and encoded_name_segment, so transformed
segments cannot match any legal raw repository name such as a hash-suffix form.
Replace the ambiguous h<8hex> alias shape with a syntactically disjoint encoding
(or encode every segment), preserve deterministic canonical naming, and add a
regression test covering the openai/java_chat versus openai/java-chat-h<digest>
collision.
---
Nitpick comments:
In `@frontend/src/lib/services/markdown.ts`:
- Around line 212-220: Optimize readEnrichmentOpening by adding an early return
when src[index] is not "{", and move the Object.keys(ENRICHMENT_KINDS)-derived
opening prefixes into a module-level precomputed collection. Reuse those
prefixes during matching instead of rebuilding strings and enumerating kinds on
each call, preserving the existing EnrichmentOpening result behavior for valid
openings.
In
`@src/main/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAlive.java`:
- Around line 104-108: Precompute the escaped model name once during
construction of EmbeddingModelKeepAlive and store it in a field, using the final
modelName value. Update recordDeferred, recordSuccess, and recordFailure to
reuse that field and remove their local CR/LF escaping.
In `@src/main/java/com/williamcallahan/javachat/web/IngestionController.java`:
- Around line 62-73: Update the IOException and RuntimeException catch blocks in
ingestLocal to pass the caught exception as the trailing argument to log.error,
matching CustomErrorController.logRequestFailure and preserving the existing
contextual messages.
🪄 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: 3617a078-eb73-43b7-b9e0-67eb3a26b3b2
📒 Files selected for processing (39)
docs/github-repository-ingestion.mdfrontend/src/lib/services/markdown.test.tsfrontend/src/lib/services/markdown.tsscripts/lib/common_qdrant.shscripts/lib/github_identity.shscripts/process_github_repo.shsrc/main/java/com/williamcallahan/javachat/cli/GitHubRepoProcessor.javasrc/main/java/com/williamcallahan/javachat/domain/ingestion/GitHubRepositoryIdentity.javasrc/main/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAlive.javasrc/main/java/com/williamcallahan/javachat/service/HybridVectorService.javasrc/main/java/com/williamcallahan/javachat/service/OpenAIStreamingService.javasrc/main/java/com/williamcallahan/javachat/service/OpenAiCompatibleEmbeddingClient.javasrc/main/java/com/williamcallahan/javachat/service/ProviderCircuitState.javasrc/main/java/com/williamcallahan/javachat/service/RateLimitHeaderParser.javasrc/main/java/com/williamcallahan/javachat/service/RateLimitState.javasrc/main/java/com/williamcallahan/javachat/service/RerankerService.javasrc/main/java/com/williamcallahan/javachat/service/StreamingAttemptContext.javasrc/main/java/com/williamcallahan/javachat/service/ingestion/SourceCodeFileIngestionProcessor.javasrc/main/java/com/williamcallahan/javachat/web/BaseController.javasrc/main/java/com/williamcallahan/javachat/web/CustomErrorController.javasrc/main/java/com/williamcallahan/javachat/web/ExceptionResponseBuilder.javasrc/main/java/com/williamcallahan/javachat/web/IngestionController.javasrc/main/java/com/williamcallahan/javachat/web/SseSupport.javasrc/test/java/com/williamcallahan/javachat/domain/ingestion/GitHubCollectionNamingScriptTest.javasrc/test/java/com/williamcallahan/javachat/domain/ingestion/GitHubRepositoryIdentityTest.javasrc/test/java/com/williamcallahan/javachat/service/EmbeddingModelKeepAliveTest.javasrc/test/java/com/williamcallahan/javachat/service/HybridVectorServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/OpenAIStreamingServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/OpenAiCompatibleEmbeddingClientTest.javasrc/test/java/com/williamcallahan/javachat/service/ProviderCircuitStateTest.javasrc/test/java/com/williamcallahan/javachat/service/RateLimitHeaderParserTest.javasrc/test/java/com/williamcallahan/javachat/service/RateLimitStateTest.javasrc/test/java/com/williamcallahan/javachat/service/RerankerServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/StreamingAttemptContextTest.javasrc/test/java/com/williamcallahan/javachat/service/ingestion/SourceCodeFileIngestionProcessorTest.javasrc/test/java/com/williamcallahan/javachat/web/CustomErrorControllerTest.javasrc/test/java/com/williamcallahan/javachat/web/ExceptionResponseBuilderTest.javasrc/test/java/com/williamcallahan/javachat/web/IngestionControllerTest.javasrc/test/java/com/williamcallahan/javachat/web/SseSupportTest.java
Comment on lines
330
to
+331
| const content = src.slice(contentStart, closeIndex); | ||
| const raw = src.slice(0, closeIndex + 2); | ||
| const raw = src.slice(0, closeIndex + ENRICHMENT_CLOSE.length); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename the new content local — it's a banned generic noun.
Line 330 introduces const content = ... inside a function that was rewritten in this diff. Per the repo's naming guideline, content is explicitly banned as a variable name, and edits touching this exact code should fix rather than perpetuate it.
As per coding guidelines, "These names and close variants are prohibited as variable, parameter, or field names: ... content, ... Use domain-specific names (Banned Generic Nouns)" and "When editing code that uses banned or generic names, rename them in the same edit ... (Legacy Fix-on-Touch)."
🐛 Proposed fix
- const content = src.slice(contentStart, closeIndex);+ const enrichmentContent = src.slice(contentStart, closeIndex);
const raw = src.slice(0, closeIndex + ENRICHMENT_CLOSE.length);
return {
type: "enrichment",
raw,
kind: opening.kind,
- content: content.trim(),+ content: enrichmentContent.trim(),
resolved: true,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| constcontent=src.slice(contentStart,closeIndex); | |
| constraw=src.slice(0,closeIndex+2); | |
| constraw=src.slice(0,closeIndex+ENRICHMENT_CLOSE.length); | |
| constenrichmentContent=src.slice(contentStart,closeIndex); | |
| constraw=src.slice(0,closeIndex+ENRICHMENT_CLOSE.length); | |
| return{ | |
| type: "enrichment", | |
| raw, | |
| kind: opening.kind, | |
| content: enrichmentContent.trim(), | |
| resolved: true, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/lib/services/markdown.ts` around lines 330 - 331, Rename the
local variable content in the rewritten function to a domain-specific name that
reflects the extracted markdown segment, and update all references to it within
that function. Leave the raw extraction logic unchanged.
Source: Coding guidelines
Comment on lines
+115
to
+120
| local repository_boundary="-" | ||
| if [[ ! "$REPOSITORY_OWNER" =~ ^[a-z0-9]+$ ]]; then | ||
| repository_boundary="_" | ||
| fi | ||
| CANONICAL_COLLECTION_NAME="github-${encoded_owner_segment}${repository_boundary}${encoded_name_segment}" | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Inspect the hash representation and the segment encoder before selecting# a backward-compatible disambiguation scheme.
rg -n -A20 -B3 \
'short_sha256\(|encode_collection_segment\(|CANONICAL_COLLECTION_NAME=' \
scripts/lib/github_identity.shRepository: WilliamAGH/java-chat
Length of output: 2877
Avoid hash-suffix aliases in collection segments.scripts/lib/github_identity.sh:58-75,115-120 emits sanitized_segment-h<8hex> for transformed names, and that shape is still a legal raw repository name. That lets openai/java_chat collide with openai/java-chat-h<digest> and share one collection, mixing vectors and prune operations. Make transformed segments syntactically disjoint from legal raw names (or encode every segment) and add a regression test.
🤖 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 `@scripts/lib/github_identity.sh` around lines 115 - 120, Update the segment
encoding logic used by the canonical collection name flow in github_identity.sh,
including the transformed-name handling around encoded_owner_segment and
encoded_name_segment, so transformed segments cannot match any legal raw
repository name such as a hash-suffix form. Replace the ambiguous h<8hex> alias
shape with a syntactically disjoint encoding (or encode every segment), preserve
deterministic canonical naming, and add a regression test covering the
openai/java_chat versus openai/java-chat-h<digest> collision.
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
Stabilizes Java Chat on the regular Gemma gateway alias and hardens the adjacent retrieval, rendering, ingestion, and error paths exposed during live dogfood.
Changes
Verification
Notes
The production generation model remains the regular gemma-4-26b-a4b alias. Structured reranking is capability-routed by the gateway to a compatible binding under that same alias.