Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/spotbugs/spotbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,14 @@
<Bug pattern="CRLF_INJECTION_LOGS"/>
<Class name="com.williamcallahan.javachat.service.ingestion.LocalDocsFileIngestionProcessor"/>
</Match>
<!--
SourceCodeFileIngestionProcessor logs repository file paths via renderPathForLog, which
escapes CR/LF before formatting; FindSecBugs cannot follow the custom sanitizer.
-->
<Match>
<Bug pattern="CRLF_INJECTION_LOGS"/>
<Class name="com.williamcallahan.javachat.service.ingestion.SourceCodeFileIngestionProcessor"/>
</Match>
<Match>
<Bug pattern="CRLF_INJECTION_LOGS"/>
<Class name="com.williamcallahan.javachat.web.ChatController"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,16 @@ public SourceFileProcessingResult process(RepositoryIngestionContext repositoryC

Optional<LocalDocsFileOutcome> validationFailure = validateFileAttributes(sourceFilePath);
if (validationFailure.isPresent()) {
if (validationFailure.get() instanceof LocalDocsFileOutcome.Skipped) {
pruneSkipIfPreviouslyIngested(repositoryContext, canonicalCollectionName, fileUrl, sourceFilePath);
}
return new SourceFileProcessingResult(validationFailure.get(), fileUrl);
}

ValidatedFileContext fileContext = buildFileContext(sourceFilePath, repositoryRoot, repositoryMetadata);
Optional<ReadableFileContent> readableContent = readAndFingerprintFile(sourceFilePath, fileContext);
if (readableContent.isEmpty()) {
pruneSkipIfPreviouslyIngested(repositoryContext, canonicalCollectionName, fileUrl, sourceFilePath);
return new SourceFileProcessingResult(LocalDocsFileOutcome.skippedFile(), fileUrl);
}
ReadableFileContent fileContent = readableContent.get();
Expand Down Expand Up @@ -254,6 +258,35 @@ private Optional<ReadableFileContent> readAndFingerprintFile(
}
}

/**
* Best-effort prune of stale prior state when a previously-ingested file is now skipped.
*
* <p>A skip (e.g. oversized, binary, or blank) returns without reindexing. When the file was
* previously ingested, its prior Qdrant chunks and local marker are left behind and would
* otherwise be shielded from orphan purging because the skip still reports the URL as active.
* This strict prune removes the stale vectors and marker so search cannot surface chunks that
* contradict the file's current on-disk bytes. A prune failure is logged and swallowed so the
* skip remains a {@link LocalDocsFileOutcome.Skipped} outcome rather than escalating to failed
* run health over best-effort cleanup.</p>
*/
private void pruneSkipIfPreviouslyIngested(
RepositoryIngestionContext repositoryContext,
String canonicalCollectionName,
String fileUrl,
Path sourceFilePath) {
if (!repositoryContext.storedFileUrls().contains(fileUrl)) {
return;
}
try {
ingestedFilePruneService.pruneCollectionFileStrict(canonicalCollectionName, fileUrl, null);
} catch (IOException pruneException) {
log.warn(
"Skip-time prune of formerly-ingested file failed (stale vectors may persist): {}",
renderPathForLog(sourceFilePath.toString()),
pruneException);
}
}

private static String sanitizeControlCharacters(String rawText, String relativePath) {
StringBuilder sanitizedTextBuilder = new StringBuilder(rawText.length());
int removedControlCharacterCount = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
Expand Down Expand Up @@ -156,6 +157,32 @@ void fileFailurePreventsOrphanPruningAndMetadataRefresh(@TempDir Path temporaryD
verifyNoInteractions(processorFixture.ingestedFilePruneService());
}

@Test
void skippedOversizedUrlStaysActiveWhileGenuinelyDeletedFileIsPurged(@TempDir Path repositoryRoot)
throws IOException {
for (int fileIndex = 0; fileIndex < ELIGIBLE_FILE_COUNT; fileIndex++) {
Files.writeString(repositoryRoot.resolve("Source" + fileIndex + ".java"), "class Source {}");
}
String oversizedUrl = "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/openai/java-chat/blob/main/package-lock.json";
String deletedUrl = "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/openai/java-chat/blob/main/Deleted.java";
ProcessorFixture processorFixture = processorFixture();
when(processorFixture.hybridVectorService().scrollAllUrlsInCollection(COLLECTION_NAME))
.thenReturn(Set.of(oversizedUrl, deletedUrl));
when(processorFixture
.fileProcessor()
.process(
any(SourceCodeFileIngestionProcessor.RepositoryIngestionContext.class),
any(Path.class)))
.thenReturn(new SourceFileProcessingResult(LocalDocsFileOutcome.skippedFile(), oversizedUrl));

processorFixture.processor().processRepository(repositoryMetadata(repositoryRoot));

verify(processorFixture.ingestedFilePruneService())
.pruneCollectionFileStrict(COLLECTION_NAME, deletedUrl, null);
verify(processorFixture.ingestedFilePruneService(), never())
.pruneCollectionFileStrict(COLLECTION_NAME, oversizedUrl, null);
}

private static GitHubRepoMetadata repositoryMetadata(Path repositoryRoot) {
return new GitHubRepoMetadata(
repositoryRoot.toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,117 @@ void processAlwaysIncludesFileUrlInResult(@TempDir Path temporaryDirectory) thro
assertTrue(sourceFileProcessing.fileUrl().startsWith("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/owner/repo/blob/main/src/Empty.java"));
}

@Test
void oversizedPreviouslyIngestedFilePrunesStalePriorState(@TempDir Path temporaryDirectory) throws IOException {
IngestionProcessorHarness harness = ingestionProcessorHarness(temporaryDirectory, "BigFile.java");
Files.writeString(
harness.sourceFilePath(),
"a".repeat((int) SourceCodeFileIngestionProcessor.MAX_FILE_SIZE_BYTES + 1),
StandardCharsets.UTF_8);

SourceFileProcessingResult sourceFileProcessing = harness.ingestionProcessor()
.process(harness.repositoryContext(Set.of(harness.sourceUrl())), harness.sourceFilePath());

assertFalse(sourceFileProcessing.outcome().processed());
assertTrue(sourceFileProcessing.outcome().failure().isEmpty());
assertEquals(harness.sourceUrl(), sourceFileProcessing.fileUrl());
verify(harness.ingestedFilePruneService())
.pruneCollectionFileStrict(TARGET_COLLECTION_NAME, harness.sourceUrl(), null);
verify(harness.fileIngestionMarkerStore(), never()).readFileIngestionRecord(anyString());
verifyNoInteractions(harness.chunkProcessingService());
}

@Test
void oversizedNeverIngestedFileSkipsWithoutPruning(@TempDir Path temporaryDirectory) throws IOException {
IngestionProcessorHarness harness = ingestionProcessorHarness(temporaryDirectory, "BigFile.java");
Files.writeString(
harness.sourceFilePath(),
"a".repeat((int) SourceCodeFileIngestionProcessor.MAX_FILE_SIZE_BYTES + 1),
StandardCharsets.UTF_8);

SourceFileProcessingResult sourceFileProcessing =
harness.ingestionProcessor().process(harness.repositoryContext(Set.of()), harness.sourceFilePath());

assertFalse(sourceFileProcessing.outcome().processed());
assertTrue(sourceFileProcessing.outcome().failure().isEmpty());
assertEquals(harness.sourceUrl(), sourceFileProcessing.fileUrl());
verifyNoInteractions(harness.ingestedFilePruneService());
verifyNoInteractions(harness.fileIngestionMarkerStore());
verifyNoInteractions(harness.chunkProcessingService());
}

@Test
void oversizedFileWithUnrelatedStoredUrlSkipsWithoutPruning(@TempDir Path temporaryDirectory) throws IOException {
IngestionProcessorHarness harness = ingestionProcessorHarness(temporaryDirectory, "BigFile.java");
Files.writeString(
harness.sourceFilePath(),
"a".repeat((int) SourceCodeFileIngestionProcessor.MAX_FILE_SIZE_BYTES + 1),
StandardCharsets.UTF_8);
String unrelatedStoredUrl = "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/openai/java-chat/blob/main/src/Unrelated.java";

SourceFileProcessingResult sourceFileProcessing = harness.ingestionProcessor()
.process(harness.repositoryContext(Set.of(unrelatedStoredUrl)), harness.sourceFilePath());

assertFalse(sourceFileProcessing.outcome().processed());
assertEquals(harness.sourceUrl(), sourceFileProcessing.fileUrl());
verifyNoInteractions(harness.ingestedFilePruneService());
}

@Test
void oversizedPruneFailureKeepsSkipOutcome(@TempDir Path temporaryDirectory) throws IOException {
IngestionProcessorHarness harness = ingestionProcessorHarness(temporaryDirectory, "BigFile.java");
Files.writeString(
harness.sourceFilePath(),
"a".repeat((int) SourceCodeFileIngestionProcessor.MAX_FILE_SIZE_BYTES + 1),
StandardCharsets.UTF_8);
Mockito.doThrow(new IOException("prune failed"))
.when(harness.ingestedFilePruneService())
.pruneCollectionFileStrict(TARGET_COLLECTION_NAME, harness.sourceUrl(), null);

SourceFileProcessingResult sourceFileProcessing = harness.ingestionProcessor()
.process(harness.repositoryContext(Set.of(harness.sourceUrl())), harness.sourceFilePath());

assertFalse(sourceFileProcessing.outcome().processed());
assertTrue(
sourceFileProcessing.outcome().failure().isEmpty(),
"Prune failure must not escalate a skip to a failure");
assertEquals(harness.sourceUrl(), sourceFileProcessing.fileUrl());
verify(harness.ingestedFilePruneService())
.pruneCollectionFileStrict(TARGET_COLLECTION_NAME, harness.sourceUrl(), null);
}

@Test
void binaryPreviouslyIngestedFilePrunesStalePriorState(@TempDir Path temporaryDirectory) throws IOException {
IngestionProcessorHarness harness = ingestionProcessorHarness(temporaryDirectory, "Binary.java");
Files.write(harness.sourceFilePath(), new byte[] {(byte) 0xFF, (byte) 0xFE, 0x00, (byte) 0xAD});

SourceFileProcessingResult sourceFileProcessing = harness.ingestionProcessor()
.process(harness.repositoryContext(Set.of(harness.sourceUrl())), harness.sourceFilePath());

assertFalse(sourceFileProcessing.outcome().processed());
assertTrue(sourceFileProcessing.outcome().failure().isEmpty());
verify(harness.ingestedFilePruneService())
.pruneCollectionFileStrict(TARGET_COLLECTION_NAME, harness.sourceUrl(), null);
verifyNoInteractions(harness.chunkProcessingService());
verify(harness.fileIngestionMarkerStore(), never()).readFileIngestionRecord(anyString());
}

@Test
void blankPreviouslyIngestedFilePrunesStalePriorState(@TempDir Path temporaryDirectory) throws IOException {
IngestionProcessorHarness harness = ingestionProcessorHarness(temporaryDirectory, "Empty.java");
Files.writeString(harness.sourceFilePath(), "", StandardCharsets.UTF_8);

SourceFileProcessingResult sourceFileProcessing = harness.ingestionProcessor()
.process(harness.repositoryContext(Set.of(harness.sourceUrl())), harness.sourceFilePath());

assertFalse(sourceFileProcessing.outcome().processed());
assertTrue(sourceFileProcessing.outcome().failure().isEmpty());
verify(harness.ingestedFilePruneService())
.pruneCollectionFileStrict(TARGET_COLLECTION_NAME, harness.sourceUrl(), null);
verifyNoInteractions(harness.chunkProcessingService());
verify(harness.fileIngestionMarkerStore(), never()).readFileIngestionRecord(anyString());
}

private static SourceCodeFileIngestionProcessor.RepositoryIngestionContext repositoryContext(
Path repositoryRoot, GitHubRepoMetadata repositoryMetadata) {
return new SourceCodeFileIngestionProcessor.RepositoryIngestionContext(
Expand Down Expand Up @@ -749,4 +860,69 @@ private SourceCodeFileIngestionProcessor.RepositoryIngestionContext repositoryCo
repositoryRoot, repositoryMetadata, storedFileUrls);
}
}

private static IngestionProcessorHarness ingestionProcessorHarness(Path temporaryDirectory, String sourceFileName)
throws IOException {
ChunkProcessingService chunkProcessingService = Mockito.mock(ChunkProcessingService.class);
HybridVectorService hybridVectorService = Mockito.mock(HybridVectorService.class);
LocalStoreService localStoreService = Mockito.mock(LocalStoreService.class);
FileIngestionMarkerStore fileIngestionMarkerStore = Mockito.mock(FileIngestionMarkerStore.class);
ContentHasher contentHasher = Mockito.mock(ContentHasher.class);
IngestedFilePruneService ingestedFilePruneService = Mockito.mock(IngestedFilePruneService.class);
SourceCodeFileIngestionProcessor ingestionProcessor = new SourceCodeFileIngestionProcessor(
new IngestionStorageServices(
hybridVectorService,
chunkProcessingService,
contentHasher,
localStoreService,
fileIngestionMarkerStore,
Mockito.mock(QdrantCollectionRouter.class)),
Mockito.mock(ProgressTracker.class),
ingestedFilePruneService);
Path repositoryRoot = temporaryDirectory.resolve("repository");
Path sourceFilePath = repositoryRoot.resolve("src").resolve(sourceFileName);
Files.createDirectories(Objects.requireNonNull(sourceFilePath.getParent(), "sourceFilePath parent"));
GitHubRepoMetadata repositoryMetadata = new GitHubRepoMetadata(
repositoryRoot.toString(),
GitHubRepositoryIdentity.of("openai", "java-chat"),
TARGET_COLLECTION_NAME,
"main",
"abcdef123456",
"MIT",
"Example repository");
String encodedSourceFileName =
URLEncoder.encode(sourceFileName, StandardCharsets.UTF_8).replace("+", "%20");
String sourceUrl = "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/openai/java-chat/blob/main/src/" + encodedSourceFileName;
return new IngestionProcessorHarness(
ingestionProcessor,
chunkProcessingService,
hybridVectorService,
localStoreService,
fileIngestionMarkerStore,
contentHasher,
ingestedFilePruneService,
repositoryRoot,
sourceFilePath,
repositoryMetadata,
sourceUrl);
}

private record IngestionProcessorHarness(
SourceCodeFileIngestionProcessor ingestionProcessor,
ChunkProcessingService chunkProcessingService,
HybridVectorService hybridVectorService,
LocalStoreService localStoreService,
FileIngestionMarkerStore fileIngestionMarkerStore,
ContentHasher contentHasher,
IngestedFilePruneService ingestedFilePruneService,
Path repositoryRoot,
Path sourceFilePath,
GitHubRepoMetadata repositoryMetadata,
String sourceUrl) {
private SourceCodeFileIngestionProcessor.RepositoryIngestionContext repositoryContext(
Set<String> storedFileUrls) {
return new SourceCodeFileIngestionProcessor.RepositoryIngestionContext(
repositoryRoot, repositoryMetadata, storedFileUrls);
}
}
}