fix(search): preserve exact Javadoc member citations#114
Open
WilliamAGH wants to merge 41 commits into
Open
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 217 files, which is 117 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (247)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request adds canonical documentation-source selection, batch Java API seed projection, release-aware Javadoc seeding, retired-vector pruning, structured Java API extraction with exact anchors, replacement-aware ingestion, and updated citation metadata handling with extensive tests. ChangesDocumentation pipeline
Java API ingestion
Validation
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 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 |
coderabbitai
Bot
added
bug
There was a problem hiding this comment.
Pull request overview
This PR updates the documentation ingestion and retrieval pipeline to preserve exact Oracle Javadoc member anchors as first-class metadata (separate from fragmentless page URLs), and makes both remote and local reindexing more deterministic/atomic by replacing URL-scoped Qdrant point sets based on stable point IDs rather than point counts.
Changes:
- Introduces structured Java API extraction (
JavaApiPageExtraction) that keeps overview text separate from anchored member sections, and threads an exactanchormetadata field through chunking → storage → citation rendering. - Replaces heuristic Javadoc URL/member rewriting with “use ingested source identity” behavior, and adds URL replacement semantics in Qdrant (
replaceUrlDocuments) plus exact point-ID coverage checks. - Adds strict retired-Java-API vector pruning (class-use pages + redirect-backed root type pages) and strengthens fetch/seed tooling around canonical selector identities.
Reviewed changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/williamcallahan/javachat/util/JavadocTypeCanonicalizerTest.java | Removes tests for heuristic member/type canonicalization (feature removed). |
| src/test/java/com/williamcallahan/javachat/StandaloneExtractionTest.java | Updates diagnostics to use extractJavaApiPage(...).combinedText(). |
| src/test/java/com/williamcallahan/javachat/service/RetrievalServiceCitationTest.java | Updates citation tests for exact-anchor metadata behavior and de-heuristicization. |
| src/test/java/com/williamcallahan/javachat/service/LocalStoreServiceTest.java | Adds coverage for deleting obsolete ingestion markers by stored hash prefixes. |
| src/test/java/com/williamcallahan/javachat/service/ingestion/MalformedUtf8LocalDocsIngestionTest.java | Adjusts malformed UTF-8 behavior expectations to account for prune boundary changes. |
| src/test/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneServiceTest.java | Adds tests for preserving replacement parsed chunks and hash-prefix marker cleanup. |
| src/test/java/com/williamcallahan/javachat/service/HybridVectorServiceTest.java | Adds tests for exact point-id coverage and atomic URL replacement semantics. |
| src/test/java/com/williamcallahan/javachat/service/HtmlContentExtractorTest.java | Adds tests for exact DOM anchors, class-use exclusion, and non-mutating extraction. |
| src/test/java/com/williamcallahan/javachat/service/DocsIngestionServiceTest.java | Adds tests for fragmentless crawl, Java API segmentation, replacement logic, and class-use deletion. |
| src/test/java/com/williamcallahan/javachat/service/ChunkProcessingServiceTest.java | Adds tests for Java API page segmentation, anchor hashing, and fragmentless URL validation. |
| src/test/java/com/williamcallahan/javachat/ExtractorQualityTest.java | Updates extraction quality checks to use the new Java API extraction shape. |
| src/main/resources/java-api-documentation-sources.manifest | Adjusts minimum HTML file thresholds for governed Java API mirrors. |
| src/main/java/com/williamcallahan/javachat/web/IngestionController.java | Routes ingestion through a new application boundary (DocumentationIngestionUseCase). |
| src/main/java/com/williamcallahan/javachat/util/JavadocTypeCanonicalizer.java | Removes heuristic type canonicalizer (no longer used). |
| src/main/java/com/williamcallahan/javachat/util/JavadocNestedTypeResolver.java | Removes heuristic nested-type URL refinement. |
| src/main/java/com/williamcallahan/javachat/util/JavadocMemberAnchorResolver.java | Removes heuristic member-anchor derivation. |
| src/main/java/com/williamcallahan/javachat/util/JavadocLinkResolver.java | Removes heuristic Javadoc link resolver facade. |
| src/main/java/com/williamcallahan/javachat/service/RetrievalService.java | Switches citations to use ingested exact-anchor metadata and canonicalizes URLs accordingly. |
| src/main/java/com/williamcallahan/javachat/service/QdrantPayloadFieldSchema.java | Adds ANCHOR_FIELD to the canonical Qdrant metadata schema. |
| src/main/java/com/williamcallahan/javachat/service/LocalStoreService.java | Adds strict deletion of obsolete chunk markers selected by stored hash prefixes. |
| src/main/java/com/williamcallahan/javachat/service/JavaApiPageExtraction.java | New record representing overview + anchored member extraction for Java API pages. |
| src/main/java/com/williamcallahan/javachat/service/JavaApiPageDisposition.java | New enum classifying whether Java API pages are indexable (e.g., class-use excluded). |
| src/main/java/com/williamcallahan/javachat/service/JavaApiAnchoredSection.java | New record preserving one authoritative member anchor + member text. |
| src/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.java | Moves to exact-point-id coverage checks and Java API structured ingestion + exclusion handling. |
| src/main/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneService.java | Adds “prune obsolete state after replacement” path (vectors + markers + parsed chunks). |
| src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java | Adds atomic URL replacement and exact point-id coverage checks via Qdrant scroll identity. |
| src/main/java/com/williamcallahan/javachat/service/HtmlContentExtractor.java | Adds structured Java API extraction (exact DOM anchors, class-use exclusion) and avoids mutation via cloning. |
| src/main/java/com/williamcallahan/javachat/service/DocumentFactory.java | Introduces canonical chunk metadata record including optional exact anchor field. |
| src/main/java/com/williamcallahan/javachat/service/DocsIngestionService.java | Implements new ingestion use case boundary; adds fragmentless crawling + URL replacement ingestion flow. |
| src/main/java/com/williamcallahan/javachat/service/ChunkProcessingService.java | Adds Java API page chunking with anchor-aware deterministic hashing and global chunk indexing. |
| src/main/java/com/williamcallahan/javachat/cli/DocumentProcessor.java | Depends on DocumentationIngestionUseCase instead of concrete service. |
| src/main/java/com/williamcallahan/javachat/application/ingestion/DocumentationIngestionUseCase.java | New application boundary interface for ingestion runs. |
| scripts/test_prune_retired_java_api_vectors.sh | New script-level test harness for retired Java API vector pruning without live Qdrant. |
| scripts/test_java_api_fetch_projection.sh | Extends tests to verify selector routing between non-Java docs and Java API projections. |
| scripts/test_documentation_seed.py | Adds tests for batch mirror-path projection and Oracle Javadoc seed constraints. |
| scripts/prune_retired_java_api_vectors.sh | New strict pruning script for retired Java API vectors (class-use + redirect-backed roots). |
| scripts/process_all_to_qdrant.sh | Runs retired Java API prune before ingestion based on effective selector. |
| scripts/oracle_javadoc_seed.py | Makes root-page seed inventory release-correct; removes class-use seed generation. |
| scripts/lib/documentation_seed_mirrors.sh | Switches to batch mirror-path projection for Java API seeds. |
| scripts/fetch_all_docs.sh | Strengthens --doc-sets to select canonical identities across both manifests before fetching. |
| scripts/documentation_seed.py | Adds --project-mirror-paths-file for batch projection and collision validation. |
| docs/pipeline-commands.md | Updates docs for the new selector semantics and adds examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+274
to
+279
| private static boolean belongsToReplacement( | ||
| ParsedChunkReference parsedChunkReference, Set<String> replacementHashSet) { | ||
| return replacementHashSet.stream() | ||
| .anyMatch(replacementChunkHash -> | ||
| replacementChunkHash.startsWith(parsedChunkReference.storedChunkHashPrefix())); | ||
| } |
Comment on lines
474
to
476
| /** | ||
| * Refines a raw document URL and gates Javadoc member anchors to {@code api-docs} metadata. | ||
| * | ||
| * Projects an authoritative ingested Javadoc anchor before applying legacy nested-type refinement. | ||
| */ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.java (2)
341-350: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReturn a typed marker-transition failure on the skipped path.
markFileIngestedwraps write failures inIllegalStateException, but this call is outside a catch block. A marker filesystem error therefore escapesprocessand can abort the batch instead of returning a failedLocalDocsFileOutcome.🤖 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/ingestion/LocalDocsFileIngestionProcessor.java` around lines 341 - 350, Update the skipped-file path in process around markFileIngested so marker filesystem failures are caught and converted into the typed failed LocalDocsFileOutcome, matching the processor’s existing failure handling. Preserve the skip logging and successful marker update behavior, and ensure the exception does not escape process or abort the batch.
361-614: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftExtract the newly added workflows from the two monolithic services.
src/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.java#L361-L614: move marker/reindex policy and replacement persistence into focused application components.src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java#L401-L484: move URL-scoped scrolling, identity reconciliation, and obsolete-point deletion into a focused Qdrant component.As per coding guidelines, “>500 LOC is a monolith. Extract pieces you touch into clean-architecture roots (Large Files).”
🤖 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/ingestion/LocalDocsFileIngestionProcessor.java` around lines 361 - 614, Extract the marker/reindex policy and replacement persistence workflow from LocalDocsFileIngestionProcessor methods inspectExistingMarker, inspectUnmarkedVectors, supersededCollectionNames, processDocuments, storeDocuments, and marker methods into focused application components, updating the processor to delegate while preserving behavior. Also update src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java lines 401-484 by extracting URL-scoped scrolling, identity reconciliation, and obsolete-point deletion into a focused Qdrant component, with HybridVectorService delegating to it; both sites require these extractions without changing existing outcomes.Source: Coding guidelines
src/main/java/com/williamcallahan/javachat/service/DocsIngestionService.java (1)
183-205: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate every hash before replacing the Qdrant corpus.
Line 197 silently skips missing hash metadata after Qdrant has already been replaced, leaving local markers inconsistent. Build and validate all marker inputs before the external mutation; fail the run if any replacement document lacks its hash.
As per coding guidelines, “Use typed exception handling patterns; propagate meaningful errors, never swallow silently.”
🤖 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/DocsIngestionService.java` around lines 183 - 205, The replaceAndMarkDocuments flow must validate every replacement document’s hash before calling hybridVectorService.replaceUrlDocuments. Update markDocumentsIngested or introduce a validation step to reject any document missing QdrantPayloadFieldSchema.HASH_FIELD, propagating a meaningful typed exception instead of silently continuing; only perform the Qdrant replacement after validation succeeds.Source: Coding guidelines
🧹 Nitpick comments (1)
src/test/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessorTest.java (1)
620-649: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the six-argument helper with a test fixture object.
The added overload extends an already long positional collaborator train, making call-site mistakes increasingly easy. Bundle these dependencies into a named test fixture or builder.
As per coding guidelines, “>4 parameters use parameter object or builder; never add 5th positional argument.”
🤖 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/ingestion/LocalDocsFileIngestionProcessorTest.java` around lines 620 - 649, Replace the overloaded six-argument createIngestionProcessor helpers with a named test fixture or builder that groups the collaborator dependencies, including quarantineService. Update all call sites to construct or configure this fixture and pass it to createIngestionProcessor, eliminating the positional parameter train while preserving the existing default mock behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/pipeline-commands.md`:
- Around line 93-97: Update the Java API refresh example in the documentation to
obtain selector values from --list-java-api-sources instead of hard-coding the
current release list. Keep the example manifest-driven so future Java API
sources are included automatically, and remove the explicit
java/java21-complete, java/java24-complete, and java/java25-complete inventory.
In
`@src/main/java/com/williamcallahan/javachat/application/ingestion/DocumentationIngestionUseCase.java`:
- Line 15: Replace the raw int ingestion limits in
DocumentationIngestionUseCase.crawlAndIngest and its related API boundaries with
validated PageLimit and FileLimit records. Add constructor validation rejecting
zero or negative values, and update CLI, tests, and direct callers to construct
these records only after transport/input validation so ingestion always receives
valid application values.
In
`@src/main/java/com/williamcallahan/javachat/service/ChunkProcessingService.java`:
- Around line 375-406: Remove Optional-valued constructor inputs from the
segment models: in ChunkProcessingService.java lines 375-406, keep
JavaApiPageSegment construction behind the overview and member factories, and in
lines 436-452 retain any nullable anchor representation privately; callers must
not supply Optional. In DocumentFactory.java lines 167-230, preserve the
withAnchor and withoutAnchor factories while encapsulating absent-anchor
handling inside their implementation.
In
`@src/main/java/com/williamcallahan/javachat/service/DocsIngestionService.java`:
- Around line 254-260: Update fetchPageSnapshot to use a named maximum
response-size constant instead of maxBodySize(0), and configure jsoup to enforce
that cap. Ensure oversized responses fail the crawl before full body
materialization, while retaining the existing snapshot preparation for responses
within the limit.
- Around line 57-65: Update the DocsIngestionService constructor to inject
AppProperties instead of the `@Value` app.docs.root-url parameter, then initialize
the root URL from AppProperties.getDocs().getRootUrl(). Remove the direct `@Value`
wiring while leaving the other constructor dependencies unchanged.
- Around line 239-244: Update the link-discovery flow in DocsIngestionService
around removeUrlFragment to normalize the configured root and candidate URLs as
parsed origin/path components rather than relying on startsWith(rootUrl). Before
queueing or continuing any crawl, validate the final response URL after
redirects and reject URLs whose host, port, or path falls outside the configured
source boundary.
In
`@src/main/java/com/williamcallahan/javachat/service/HtmlContentExtractor.java`:
- Around line 28-33: Extract the Java API-specific page classification,
overview, anchor parsing, and member extraction logic from HtmlContentExtractor
into a new focused JavaApiPageExtractor under the project’s canonical
architecture root, placing its extraction contract alongside it or in an
adjacent canonical location. Update HtmlContentExtractor to delegate to this
component while retaining only generic HTML cleanup responsibilities, and
preserve the existing public behavior.
In
`@src/main/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneService.java`:
- Around line 120-132: In the cleanup flow containing parsedChunkReferences and
obsoleteChunkHashes, invoke deleteObsoleteChunkIngestionMarkersByHashPrefixes
before deleteChunkIngestionMarkers so ambiguous prefixes fail closed before any
exact-marker deletion occurs. Preserve the existing arguments and subsequent
deleteObsoleteParsedChunks call.
In
`@src/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.java`:
- Around line 287-326: The ingestion flow around chunkingOutcome.documents()
currently accepts partial results; update it to trigger the existing
force-processing path whenever chunkingOutcome.skippedChunks() is greater than
zero, not only when chunkingOutcome.skippedAllChunks() is true. Ensure the
complete forced document set is processed and its hashes recorded, while
retaining the current fast path for outcomes with no skipped chunks.
In
`@src/main/java/com/williamcallahan/javachat/service/JavaApiAnchoredSection.java`:
- Around line 14-30: Make JavadocMemberAnchor the single validated domain type
for exact DOM identifiers: add and validate this record in
JavaApiAnchoredSection.java, then change JavaApiAnchoredSection to accept it. In
ChunkProcessingService.java lines 464-471, remove the local anchor validator and
construct/use JavadocMemberAnchor. In DocumentFactory.java lines 240-245, remove
its duplicate validation and serialize the canonical anchor’s string value.
---
Outside diff comments:
In
`@src/main/java/com/williamcallahan/javachat/service/DocsIngestionService.java`:
- Around line 183-205: The replaceAndMarkDocuments flow must validate every
replacement document’s hash before calling
hybridVectorService.replaceUrlDocuments. Update markDocumentsIngested or
introduce a validation step to reject any document missing
QdrantPayloadFieldSchema.HASH_FIELD, propagating a meaningful typed exception
instead of silently continuing; only perform the Qdrant replacement after
validation succeeds.
In
`@src/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.java`:
- Around line 341-350: Update the skipped-file path in process around
markFileIngested so marker filesystem failures are caught and converted into the
typed failed LocalDocsFileOutcome, matching the processor’s existing failure
handling. Preserve the skip logging and successful marker update behavior, and
ensure the exception does not escape process or abort the batch.
- Around line 361-614: Extract the marker/reindex policy and replacement
persistence workflow from LocalDocsFileIngestionProcessor methods
inspectExistingMarker, inspectUnmarkedVectors, supersededCollectionNames,
processDocuments, storeDocuments, and marker methods into focused application
components, updating the processor to delegate while preserving behavior. Also
update
src/main/java/com/williamcallahan/javachat/service/HybridVectorService.java
lines 401-484 by extracting URL-scoped scrolling, identity reconciliation, and
obsolete-point deletion into a focused Qdrant component, with
HybridVectorService delegating to it; both sites require these extractions
without changing existing outcomes.
---
Nitpick comments:
In
`@src/test/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessorTest.java`:
- Around line 620-649: Replace the overloaded six-argument
createIngestionProcessor helpers with a named test fixture or builder that
groups the collaborator dependencies, including quarantineService. Update all
call sites to construct or configure this fixture and pass it to
createIngestionProcessor, eliminating the positional parameter train while
preserving the existing default mock 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: 7bf67b10-7722-4464-bfd6-25cac31e07f3
📒 Files selected for processing (43)
docs/pipeline-commands.mdscripts/documentation_seed.pyscripts/fetch_all_docs.shscripts/lib/documentation_seed_mirrors.shscripts/oracle_javadoc_seed.pyscripts/process_all_to_qdrant.shscripts/prune_retired_java_api_vectors.shscripts/test_documentation_seed.pyscripts/test_java_api_fetch_projection.shscripts/test_prune_retired_java_api_vectors.shsrc/main/java/com/williamcallahan/javachat/application/ingestion/DocumentationIngestionUseCase.javasrc/main/java/com/williamcallahan/javachat/cli/DocumentProcessor.javasrc/main/java/com/williamcallahan/javachat/service/ChunkProcessingService.javasrc/main/java/com/williamcallahan/javachat/service/DocsIngestionService.javasrc/main/java/com/williamcallahan/javachat/service/DocumentFactory.javasrc/main/java/com/williamcallahan/javachat/service/HtmlContentExtractor.javasrc/main/java/com/williamcallahan/javachat/service/HybridVectorService.javasrc/main/java/com/williamcallahan/javachat/service/JavaApiAnchoredSection.javasrc/main/java/com/williamcallahan/javachat/service/JavaApiPageDisposition.javasrc/main/java/com/williamcallahan/javachat/service/JavaApiPageExtraction.javasrc/main/java/com/williamcallahan/javachat/service/LocalStoreService.javasrc/main/java/com/williamcallahan/javachat/service/QdrantPayloadFieldSchema.javasrc/main/java/com/williamcallahan/javachat/service/RetrievalService.javasrc/main/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneService.javasrc/main/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessor.javasrc/main/java/com/williamcallahan/javachat/util/JavadocLinkResolver.javasrc/main/java/com/williamcallahan/javachat/util/JavadocMemberAnchorResolver.javasrc/main/java/com/williamcallahan/javachat/util/JavadocNestedTypeResolver.javasrc/main/java/com/williamcallahan/javachat/util/JavadocTypeCanonicalizer.javasrc/main/java/com/williamcallahan/javachat/web/IngestionController.javasrc/main/resources/java-api-documentation-sources.manifestsrc/test/java/com/williamcallahan/javachat/ExtractorQualityTest.javasrc/test/java/com/williamcallahan/javachat/StandaloneExtractionTest.javasrc/test/java/com/williamcallahan/javachat/service/ChunkProcessingServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/DocsIngestionServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/HtmlContentExtractorTest.javasrc/test/java/com/williamcallahan/javachat/service/HybridVectorServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/LocalStoreServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/RetrievalServiceCitationTest.javasrc/test/java/com/williamcallahan/javachat/service/ingestion/IngestedFilePruneServiceTest.javasrc/test/java/com/williamcallahan/javachat/service/ingestion/LocalDocsFileIngestionProcessorTest.javasrc/test/java/com/williamcallahan/javachat/service/ingestion/MalformedUtf8LocalDocsIngestionTest.javasrc/test/java/com/williamcallahan/javachat/util/JavadocTypeCanonicalizerTest.java
💤 Files with no reviewable changes (5)
- src/main/java/com/williamcallahan/javachat/util/JavadocMemberAnchorResolver.java
- src/main/java/com/williamcallahan/javachat/util/JavadocLinkResolver.java
- src/main/java/com/williamcallahan/javachat/util/JavadocNestedTypeResolver.java
- src/test/java/com/williamcallahan/javachat/util/JavadocTypeCanonicalizerTest.java
- src/main/java/com/williamcallahan/javachat/util/JavadocTypeCanonicalizer.java
Comment on lines
+120
to
+132
| List<ParsedChunkReference> parsedChunkReferences = readParsedChunkReferences(sourceUrl); | ||
| Set<String> obsoleteChunkHashes = new LinkedHashSet<>(); | ||
| if (previousFileRecord != null) { | ||
| obsoleteChunkHashes.addAll(previousFileRecord.chunkHashes()); | ||
| } | ||
| obsoleteChunkHashes.removeAll(replacementHashSet); | ||
| if (!obsoleteChunkHashes.isEmpty()) { | ||
| localStoreService.deleteChunkIngestionMarkers(List.copyOf(obsoleteChunkHashes)); | ||
| } | ||
| localStoreService.deleteObsoleteChunkIngestionMarkersByHashPrefixes( | ||
| obsoleteStoredHashPrefixes(parsedChunkReferences, replacementHashSet), | ||
| List.copyOf(replacementChunkHashes)); | ||
| deleteObsoleteParsedChunks(parsedChunkReferences, replacementHashSet); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Check prefix ambiguity before deleting any exact markers.
deleteChunkIngestionMarkers mutates state before deleteObsoleteChunkIngestionMarkersByHashPrefixes can reject an ambiguous prefix. If ambiguity is found, cleanup has already partially completed. Invoke the prefix-based fail-closed operation first, then delete remaining exact hashes.
Proposed ordering
- if (!obsoleteChunkHashes.isEmpty()) {
- localStoreService.deleteChunkIngestionMarkers(List.copyOf(obsoleteChunkHashes));
- }
localStoreService.deleteObsoleteChunkIngestionMarkersByHashPrefixes(
obsoleteStoredHashPrefixes(parsedChunkReferences, replacementHashSet),
List.copyOf(replacementChunkHashes));
+ if (!obsoleteChunkHashes.isEmpty()) {
+ localStoreService.deleteChunkIngestionMarkers(List.copyOf(obsoleteChunkHashes));
+ }📝 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
| List<ParsedChunkReference> parsedChunkReferences = readParsedChunkReferences(sourceUrl); | |
| Set<String> obsoleteChunkHashes = new LinkedHashSet<>(); | |
| if (previousFileRecord != null) { | |
| obsoleteChunkHashes.addAll(previousFileRecord.chunkHashes()); | |
| } | |
| obsoleteChunkHashes.removeAll(replacementHashSet); | |
| if (!obsoleteChunkHashes.isEmpty()) { | |
| localStoreService.deleteChunkIngestionMarkers(List.copyOf(obsoleteChunkHashes)); | |
| } | |
| localStoreService.deleteObsoleteChunkIngestionMarkersByHashPrefixes( | |
| obsoleteStoredHashPrefixes(parsedChunkReferences, replacementHashSet), | |
| List.copyOf(replacementChunkHashes)); | |
| deleteObsoleteParsedChunks(parsedChunkReferences, replacementHashSet); | |
| List<ParsedChunkReference> parsedChunkReferences = readParsedChunkReferences(sourceUrl); | |
| Set<String> obsoleteChunkHashes = new LinkedHashSet<>(); | |
| if (previousFileRecord != null) { | |
| obsoleteChunkHashes.addAll(previousFileRecord.chunkHashes()); | |
| } | |
| obsoleteChunkHashes.removeAll(replacementHashSet); | |
| localStoreService.deleteObsoleteChunkIngestionMarkersByHashPrefixes( | |
| obsoleteStoredHashPrefixes(parsedChunkReferences, replacementHashSet), | |
| List.copyOf(replacementChunkHashes)); | |
| if (!obsoleteChunkHashes.isEmpty()) { | |
| localStoreService.deleteChunkIngestionMarkers(List.copyOf(obsoleteChunkHashes)); | |
| } | |
| deleteObsoleteParsedChunks(parsedChunkReferences, replacementHashSet); |
🤖 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/ingestion/IngestedFilePruneService.java`
around lines 120 - 132, In the cleanup flow containing parsedChunkReferences and
obsoleteChunkHashes, invoke deleteObsoleteChunkIngestionMarkersByHashPrefixes
before deleteChunkIngestionMarkers so ambiguous prefixes fail closed before any
exact-marker deletion occurs. Preserve the existing arguments and subsequent
deleteObsoleteParsedChunks call.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1263e6496c
ℹ️ 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".
| @@ -247,41 +287,43 @@ public LocalDocsFileOutcome process(Path root, Path file) { | |||
| List<Document> documents = chunkingOutcome.documents(); | |||
| if (!documents.isEmpty()) { | |||
There was a problem hiding this comment.
When local chunk-hash markers cause only some chunks to be skipped but Qdrant is missing the skipped points, such as after a collection reset that leaves data/index intact, this early return upserts only the non-skipped documents and then records the full chunk-hash inventory for the file. The run reports success while the URL is missing every skipped chunk until a later ingestion happens to detect the exact-ID mismatch, so the local path needs the same complete-replacement check used by the crawler before marking the file complete.
Useful? React with 👍 / 👎.
A canceled bounded-elastic retrieval could finish after the test that created it, leaking an unrelated pipeline error into the next test's global log capture. Synchronize the actual retrieval task before disposing the stream so suite order and machine timing cannot change the outcome.
The test's real sleep could consume the entire shared query deadline under loaded CI scheduling. Sequential dispatch timestamps already verify that later queries receive the remaining budget without making the assertion flaky.
The guided lesson page header already owns the lesson title, so remove each curated document's duplicate opening heading. Guard the content resources so a second primary heading cannot be reintroduced.
Track malformed URL-scoped parsed files as obsolete cleanup candidates while keeping unverified hashes away from ingestion-marker deletion. This prevents corrupt local artifacts from surviving every replacement run.
Select the runtime constructor explicitly when Spring sees the service's test constructor. Cover CLI-profile context creation so batch ingestion cannot fail before document processing begins.
Keep GitHub synchronization with its direct runtime owner and fail when active-generation discovery or repository metadata cannot be verified.
Own source selection, stable-version validation, citation provenance, and configurable documentation roots at their runtime boundaries.
Align the OpenAI client, Qdrant client and server, and gRPC stack while isolating local Qdrant storage for the new embedding generation.
Use Qwen 4B at 2,560 native dimensions with intent-specific gateway tiers, strict response validation, and no competing remote provider path.
Require exact 4B/2,560 core schemas and active GitHub prefixes, and keep readiness down when governed collection validation fails.
Fail closed on provider or file errors, require exact per-source Qdrant postconditions, and preserve prior generation markers and points until replacement succeeds.
Align local, CI, deployment, ingestion, and monitoring guidance with the shared gateway and isolated Qdrant generation contract.
Treat all as a selector expansion owned by the full-source fetch path so operators can request the complete canonical corpus without naming each source. - keep quick mirrors excluded from all - reject all when mixed with named selectors - cover the routing contract without duplicating the source inventory
Generate source identity hashes through the already-required Python runtime so staged fetch publication works consistently on macOS and Linux hosts. - remove the platform-specific shasum dependency - assert the stable SHA-256 staging identity in the fetch projection test
Readiness now intentionally covers Qdrant, so the container smoke must provide the same pinned dependency instead of waiting on an impossible localhost link. - start Qdrant 1.18.3 on the CI Compose network - connect the application container through the Qdrant service name - remove the ephemeral dependency volume during always-run cleanup
Prevent slow SDK setup from accumulating burst credit before an embedding request begins. Keep admission, pacing, async completion, and caller timeouts within the gateway limits. - Space every consecutive SDK embedding dispatch by tier - Retain concurrency permits until in-flight futures complete - Cover pacing deadlines, interruption, timeout, and permit recovery
The lesson source panel could show unrelated vector-search matches instead of the official links authored into the lesson. Derive static lesson sources from the canonical Markdown AST and preserve inline-code link labels. - remove Qdrant discovery from static lesson citations - retain authored source order, URLs, and titles - cover the Pattern Matching JLS links and inline code labels
Require an explicit Responses API completion event before recording stream success, preserve refusal text, and classify terminal server, rate-limit, output-limit, and content-filter states. Keep visible partial answers while rendering terminal failures separately and treat invisible-only output as empty.
Extract plural Java release requests, search each requested API generation under exact source constraints, and preserve per-release evidence through reranking and final prompt limits. Keep guided prompts scoped to the requested releases while leaving non-Java lesson retrieval unconstrained by incidental Java version mentions.
Reserve the floating chat control footprint in the lesson scrollport's programmatic target area so expanded source links are not obscured on mobile. Correct the modules lesson's duplicate greeting punctuation.
A query naming an exact Java method overload (for example java.util.List.of(E, E)) was routed through the general reranker, which scores candidates by semantic similarity and can rank a package-use or overview page above the authoritative overload page. Multi-release comparisons could therefore answer from documents that never described the requested overload. Exact-overload queries now retrieve Java API citation candidates directly, order them with CitationCandidateRanker, and skip reranking entirely. Missing evidence raises instead of silently degrading to whatever the reranker surfaced, and each requested release must supply its own evidence. Detection is gated on the retrieval constraint targeting a Java API doc set, so non-Java scopes that merely mention Java syntax keep the ordinary reranked path. - Add requiresExactJavaOverloadEvidence to gate the exact-overload path on JavaApiMethodSelector plus the constraint's Java API doc sets - Add requireExactOverloadEvidence to fail loudly when a requested release has no official overload evidence - Retrieve and order exact-overload candidates in retrieveCandidates and bypass rerankerService when building prompt documents - Cover multi-release exact-overload grounding and the non-Java scope exclusion in RetrievalServiceTest
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Production migration
After deployment, run the Java 21/24/25 mirror ingestion. The pipeline prunes retired class-use/root redirect vectors first, then reindexes all governed Javadoc pages with exact anchors.
Verification