diff --git a/frontend b/frontend index 4d314b5b..7db82a63 160000 --- a/frontend +++ b/frontend @@ -1 +1 @@ -Subproject commit 4d314b5b70e0bea43feefe8597f9728db57098d1 +Subproject commit 7db82a639b7fa0c97b483217fe43cc379912b430 diff --git a/java-ecosystem/libs/analysis-api/src/main/java/org/rostilos/codecrow/analysisapi/rag/RagOperationsService.java b/java-ecosystem/libs/analysis-api/src/main/java/org/rostilos/codecrow/analysisapi/rag/RagOperationsService.java index 3b882325..7dc977ec 100644 --- a/java-ecosystem/libs/analysis-api/src/main/java/org/rostilos/codecrow/analysisapi/rag/RagOperationsService.java +++ b/java-ecosystem/libs/analysis-api/src/main/java/org/rostilos/codecrow/analysisapi/rag/RagOperationsService.java @@ -100,23 +100,20 @@ default boolean isMultiBranchEnabled(Project project) { } /** - * Check if a branch should have indexed context based on project configuration. - * Branch indexes are created for branches that match branchPushPatterns in BranchAnalysisConfig. - * + * Check if a branch is explicitly configured for a retained RAG index. + * Branch analysis configuration is a separate concern and never grants RAG + * snapshot ownership. + * * @param project The project to check * @param branchName The branch name to evaluate - * @return true if branch should have indexed context + * @return true if the branch is explicitly configured for retained indexed context */ default boolean shouldHaveBranchIndex(Project project, String branchName) { var config = project.getConfiguration(); if (config == null || config.ragConfig() == null) { return false; } - // Get branch push patterns from branch analysis config - var branchPushPatterns = config.branchAnalysis() != null - ? config.branchAnalysis().branchPushPatterns() - : null; - return config.ragConfig().shouldHaveBranchIndex(branchName, branchPushPatterns); + return config.ragConfig().shouldHaveBranchIndex(branchName); } /** diff --git a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java index cb735f3c..cd5992db 100644 --- a/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java +++ b/java-ecosystem/libs/analysis-engine/src/main/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessor.java @@ -944,7 +944,7 @@ private void performIncrementalRagUpdate(BranchProcessRequest request, Project p "RAG module not deployed — skipping incremental update"); return; } - if (commitDiff == null || commitDiff.isBlank()) { + if (scopedOnly && (commitDiff == null || commitDiff.isBlank())) { log.info("Skipping RAG incremental update - no scoped files require an update"); EventNotificationEmitter.emitStatus(consumer, "rag_skipped", "No scoped files require a RAG update"); @@ -966,19 +966,19 @@ private void performIncrementalRagUpdate(BranchProcessRequest request, Project p return; } - String targetBranch = request.getTargetBranchName(); - String baseBranch = ragOperationsService.getBaseBranch(project); + String targetBranch = request.getTargetBranchName(); + String baseBranch = ragOperationsService.getBaseBranch(project); - if (!targetBranch.equals(baseBranch) - && !ragOperationsService.shouldHaveBranchIndex(project, targetBranch)) { - log.info("Skipping RAG update for non-retained branch: project={}, branch={}", - project.getId(), targetBranch); - EventNotificationEmitter.emitStatus(consumer, "rag_skipped", - "Branch is analyzed but is not configured as a retained RAG branch"); - return; - } + if (!targetBranch.equals(baseBranch) + && !ragOperationsService.shouldHaveBranchIndex(project, targetBranch)) { + log.info("Skipping RAG update for non-retained branch: project={}, branch={}", + project.getId(), targetBranch); + EventNotificationEmitter.emitStatus(consumer, "rag_skipped", + "Branch is analyzed but is not configured as a retained RAG branch"); + return; + } - // Health check: verify RAG pipeline is reachable before starting + // Health check: verify RAG pipeline is reachable before starting if (!ragOperationsService.isRagPipelineHealthy()) { log.warn("RAG pipeline is not reachable — skipping incremental update for project={}", project.getId()); @@ -1013,10 +1013,11 @@ private void performIncrementalRagUpdate(BranchProcessRequest request, Project p } } - log.info("RAG update completed for project={}, branch={}, commit={}", + // RagOperationsService owns the precise terminal event. A boolean true + // also covers an already-current revision, so translating it into a + // generic "updated" event here would be a false success report. + log.info("RAG reconciliation completed for project={}, branch={}, commit={}", project.getId(), targetBranch, request.getCommitHash()); - EventNotificationEmitter.emitStatus(consumer, "rag_update_complete", - "RAG index updated successfully for branch: " + targetBranch); } catch (Exception e) { log.warn("RAG incremental update failed (non-critical): {}", e.getMessage()); EventNotificationEmitter.emitStatus(consumer, "rag_update_failed", diff --git a/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessorTest.java b/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessorTest.java index 8ba074a3..3d6006b1 100644 --- a/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessorTest.java +++ b/java-ecosystem/libs/analysis-engine/src/test/java/org/rostilos/codecrow/analysisengine/processor/analysis/BranchAnalysisProcessorTest.java @@ -838,6 +838,67 @@ void shouldNotEmitRagSuccessAfterIncrementalFailure() { assertThat(event).containsEntry("state", "rag_update_complete")); } + @Test + @DisplayName("should let RAG service report the precise successful outcome") + void shouldNotSynthesizeRagUpdatedFromBooleanSuccess() { + BranchProcessRequest request = createRequest(); + request.commitHash = "current-commit"; + request.targetBranchName = "main"; + String rawDiff = "diff --git a/f.java b/f.java\n+x\n"; + List> events = new ArrayList<>(); + + when(project.getId()).thenReturn(1L); + when(ragOperationsService.isRagEnabled(project)).thenReturn(true); + when(ragOperationsService.isRagIndexReady(project)).thenReturn(true); + when(ragOperationsService.isRagPipelineHealthy()).thenReturn(true); + when(ragOperationsService.getBaseBranch(project)).thenReturn("main"); + when(ragOperationsService.triggerIncrementalUpdate( + eq(project), eq("main"), eq("current-commit"), eq(rawDiff), any())) + .thenReturn(true); + + ReflectionTestUtils.invokeMethod( + processor, + "performIncrementalRagUpdate", + request, + project, + rawDiff, + (Consumer>) events::add, + false); + + assertThat(events) + .noneSatisfy(event -> + assertThat(event).containsEntry("state", "rag_update_complete")); + } + + @Test + @DisplayName("should reconcile an empty base-branch diff through the durable RAG operation") + void shouldDelegateEmptyBaseBranchDiff() { + BranchProcessRequest request = createRequest(); + request.commitHash = "empty-range-commit"; + request.targetBranchName = "main"; + + when(project.getId()).thenReturn(1L); + when(ragOperationsService.isRagEnabled(project)).thenReturn(true); + when(ragOperationsService.isRagIndexReady(project)).thenReturn(true); + when(ragOperationsService.isRagPipelineHealthy()).thenReturn(true); + when(ragOperationsService.getBaseBranch(project)).thenReturn("main"); + when(ragOperationsService.triggerIncrementalUpdate( + eq(project), eq("main"), eq("empty-range-commit"), eq(""), any())) + .thenReturn(true); + + ReflectionTestUtils.invokeMethod( + processor, + "performIncrementalRagUpdate", + request, + project, + "", + (Consumer>) ignored -> { }, + false); + + verify(ragOperationsService).triggerIncrementalUpdate( + eq(project), eq("main"), eq("empty-range-commit"), eq(""), any()); + } + @Test @DisplayName("should call updateBranchIndex for non-main branch RAG update") void shouldCallUpdateBranchIndexForNonMainBranch() throws Exception { diff --git a/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/model/project/config/RagConfig.java b/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/model/project/config/RagConfig.java index 43132e6e..c1737546 100644 --- a/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/model/project/config/RagConfig.java +++ b/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/model/project/config/RagConfig.java @@ -20,7 +20,7 @@ * source changes come from the exact PR overlay rather than a second branch. * - branchRetentionDays: how long to keep branch index metadata before auto-cleanup (default: 90 days) * - indexedBranches: explicit non-primary branches whose complete snapshots are retained. - * A null/empty value preserves the legacy branchPushPatterns interpretation. + * Branch analysis patterns do not implicitly retain RAG snapshots. * - transientBranchIndexesEnabled: whether an analyzed PR target that is not retained * may receive a revision-pinned temporary snapshot. */ @@ -58,8 +58,9 @@ public RagConfig(boolean enabled, String branch, List includePatterns, L } /** - * Backward-compatible constructor for configurations written before explicit - * retained and transient branch ownership was introduced. + * Compatibility constructor for configurations written before explicit + * retained and transient branch ownership was introduced. Such configurations + * retain no non-primary RAG branches until they are selected explicitly. */ public RagConfig( boolean enabled, @@ -88,11 +89,6 @@ public int getEffectiveBranchRetentionDays() { return branchRetentionDays != null ? branchRetentionDays : DEFAULT_BRANCH_RETENTION_DAYS; } - public boolean hasExplicitIndexedBranches() { - return indexedBranches != null && indexedBranches.stream() - .anyMatch(value -> value != null && !value.isBlank()); - } - @JsonIgnore public List getEffectiveIndexedBranches() { if (indexedBranches == null) { @@ -111,36 +107,16 @@ public boolean isTransientBranchIndexesEnabled() { } /** - * Check if a branch should have indexed context based on branchPushPatterns. - * @param branchName the branch to check - * @param branchPushPatterns patterns from BranchAnalysisConfig - * @return true if branch matches any pattern and multi-branch is enabled + * Check whether a branch is explicitly configured for a retained RAG index. + * Branch-analysis patterns intentionally have no effect on this decision. + * + * @param branchName the exact branch name to check + * @return true if the branch is explicitly retained and multi-branch indexing is enabled */ - public boolean shouldHaveBranchIndex(String branchName, List branchPushPatterns) { + public boolean shouldHaveBranchIndex(String branchName) { if (!isMultiBranchEnabled() || branchName == null || branchName.isBlank()) { return false; } - if (hasExplicitIndexedBranches()) { - return getEffectiveIndexedBranches().contains(branchName.trim()); - } - if (branchPushPatterns == null || branchPushPatterns.isEmpty()) { - return false; - } - return branchPushPatterns.stream() - .anyMatch(pattern -> matchesBranchPattern(branchName, pattern)); - } - - /** - * Match a branch name against a glob pattern. - */ - public static boolean matchesBranchPattern(String branchName, String pattern) { - if (pattern == null || branchName == null) return false; - // Convert glob pattern to regex - String regex = pattern - .replace(".", "\\.") - .replace("**", "§§") // Temp placeholder for ** - .replace("*", "[^/]*") - .replace("§§", ".*"); - return branchName.matches(regex); + return getEffectiveIndexedBranches().contains(branchName.trim()); } } diff --git a/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParser.java b/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParser.java index e481c496..12ae40a6 100644 --- a/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParser.java +++ b/java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParser.java @@ -10,27 +10,17 @@ public final class QaDocContentParser { public static final String TEST_CASES_START = ""; + public static final String TEST_CASES_CONTENT = ""; public static final String TEST_CASES_END = ""; + public static final String ENVIRONMENT_START = ""; + public static final String ENVIRONMENT_CONTENT = ""; + public static final String ENVIRONMENT_END = ""; private static final Pattern SCENARIO_PATTERN = Pattern.compile( "(?m)^\\s*\\*\\*(.+?)\\*\\*\\s*\\((HIGH|MEDIUM|LOW)\\)[^\\r\\n]*$", Pattern.CASE_INSENSITIVE ); private static final Pattern HEADING_PATTERN = Pattern.compile("(?m)^\\s*(#{2,6})\\s+(.+?)\\s*$"); - private static final Pattern LEGACY_SECTION_PATTERN = Pattern.compile( - "(?im)^\\s*(#{2,4})\\s+(?:\\d+\\.\\s*)?Test Scenarios(?:\\s+by Area)?\\s*$" - ); - private static final Pattern NUMBERED_SECTION_HEADING = Pattern.compile("^\\d+\\.\\s+.+$"); - private static final Pattern KNOWN_LATER_SECTION_HEADING = Pattern.compile( - "^(?:Edge Cases(?: and Negative Testing)?|Negative Testing|Regression Risks|" - + "Environment(?: and Setup Notes)?|Setup Notes).*$", - Pattern.CASE_INSENSITIVE - ); - private static final Pattern ENVIRONMENT_SECTION_HEADING = Pattern.compile( - "^(?:6\\.\\s+.+|(?:\\d+\\.\\s*)?(?:Environment and Setup Notes|" - + "Setup and Environment Notes|Environment Setup Notes|Environment Notes|Setup Notes))$", - Pattern.CASE_INSENSITIVE - ); private static final Pattern GENERATED_QA_FOOTER = Pattern.compile( "(?ms)\\n\\s*---\\s*\\R\\s*\\*🐦 Generated by \\[CodeCrow]\\([^\\r\\n]+\\) " + "QA Auto-Documentation\\*\\s*\\R\\s*" @@ -44,252 +34,210 @@ public static QaDocContent parse(String markdown) { String source = markdown == null ? "" : GENERATED_QA_FOOTER.matcher(markdown.trim()).replaceFirst("").trim(); - Section section = findTestCaseSection(source); - String withoutTestCases; - List testCases; - if (section == null) { - withoutTestCases = source; - testCases = parseTestCases(source); - } else { - withoutTestCases = (source.substring(0, section.start()) - + "\n\n" - + source.substring(section.end())) - .replace(TEST_CASES_START, "") - .replace(TEST_CASES_END, ""); - testCases = parseTestCases(section.content()); - } - - Section environmentSection = findEnvironmentSection(withoutTestCases); - String overview = environmentSection == null - ? normalizeDocumentPart(withoutTestCases) - : normalizeDocumentPart( - withoutTestCases.substring(0, environmentSection.start()) - + "\n\n" - + withoutTestCases.substring(environmentSection.end())); - String environment = environmentSection == null + MarkedBlock testCasesBlock = findMarkedBlock( + source, + TEST_CASES_START, + TEST_CASES_CONTENT, + TEST_CASES_END); + MarkedBlock environmentBlock = findMarkedBlock( + source, + ENVIRONMENT_START, + ENVIRONMENT_CONTENT, + ENVIRONMENT_END); + + List testCases = testCasesBlock == null + ? List.of() + : parseTestCases(testCasesBlock.contentMarkdown()); + String environment = environmentBlock == null ? null - : stripFirstHeading(environmentSection.content()); + : normalizeNullablePart(environmentBlock.contentMarkdown()); - return new QaDocContent(overview, testCases, normalizeNullablePart(environment)); + List excludedBlocks = new ArrayList<>(); + if (testCasesBlock != null) { + excludedBlocks.add(testCasesBlock); + } + if (environmentBlock != null) { + excludedBlocks.add(environmentBlock); + } + excludedBlocks.sort((left, right) -> Integer.compare(left.start(), right.start())); + + return new QaDocContent( + normalizeDocumentPart(removeBlocks(source, excludedBlocks)), + testCases, + environment); } /** - * Parses only an explicitly marked section for unauthenticated disclosure. - * Legacy heading inference is intentionally excluded from this boundary. + * Parses only the exact test-case sentinel block for unauthenticated disclosure. + * No heading text is inspected or inferred at this authorization boundary. */ public static List parseMarkedTestCases(String markdown) { + MarkedBlock block = findMarkedBlock( + markdown == null ? "" : markdown.trim(), + TEST_CASES_START, + TEST_CASES_CONTENT, + TEST_CASES_END); + return block == null + ? List.of() + : parseStructuredTestCases(block.contentMarkdown()); + } + + /** Returns whether both shareable sections satisfy the exact sentinel contract. */ + public static boolean hasCompleteShareableSections(String markdown) { String source = markdown == null ? "" : markdown.trim(); - MarkedSection section = findMarkedSection(source); - if (section == null) { - return List.of(); - } - return parseStructuredTestCases( - source.substring(section.testContentStart(), section.testContentEnd()).trim()); + MarkedBlock testCasesBlock = findMarkedBlock( + source, + TEST_CASES_START, + TEST_CASES_CONTENT, + TEST_CASES_END); + MarkedBlock environmentBlock = findMarkedBlock( + source, + ENVIRONMENT_START, + ENVIRONMENT_CONTENT, + ENVIRONMENT_END); + return testCasesBlock != null + && !parseStructuredTestCases(testCasesBlock.contentMarkdown()).isEmpty() + && environmentBlock != null + && testCasesBlock.end() <= environmentBlock.start() + && !environmentBlock.contentMarkdown().isBlank(); } /** - * Keeps the compact Jira-facing parts of the rendered QA document while - * replacing the broad test-case and environment/setup bodies with their - * respective public-preview links. Original section headings and the - * generated ownership footer are retained. + * Keeps the compact Jira-facing parts of the rendered QA document while replacing + * each exact shareable block with its opaque localized heading and preview link. */ public static String replaceShareableSections( String markdown, String testCasesReplacement, String environmentReplacement) { - String withTestCasesReplaced = replaceMarkedTestCaseBody(markdown, testCasesReplacement); - return replaceEnvironmentBody(withTestCasesReplaced, environmentReplacement); - } - - private static String replaceMarkedTestCaseBody(String markdown, String replacementMarkdown) { String source = markdown == null ? "" : markdown.trim(); - String replacement = replacementMarkdown == null ? "" : replacementMarkdown.trim(); - if (replacement.isBlank()) { - throw new IllegalArgumentException("Test-case replacement is required."); - } - - MarkedSection section = findMarkedSection(source); - if (section == null) { - throw new IllegalArgumentException("A marked QA test-case section is required."); - } - - String laterSections = source - .substring(section.testContentEnd(), section.markerEnd()) - .trim(); - String afterMarker = source - .substring(section.markerEnd() + TEST_CASES_END.length()) - .trim(); - - StringBuilder result = new StringBuilder(source - .substring(0, section.contentStart()).stripTrailing()) - .append("\n") - .append(section.heading()) - .append("\n\n") - .append(replacement) - .append("\n") - .append(TEST_CASES_END); - if (!laterSections.isBlank()) { - result.append("\n\n").append(laterSections); - } - if (!afterMarker.isBlank()) { - result.append("\n\n").append(afterMarker); - } - return result.toString().trim(); + String testCasesLink = requireReplacement(testCasesReplacement, "Test-case"); + String environmentLink = requireReplacement(environmentReplacement, "Environment"); + + MarkedBlock testCasesBlock = requireMarkedBlock( + source, + TEST_CASES_START, + TEST_CASES_CONTENT, + TEST_CASES_END, + "test-case"); + source = replaceBlock(source, testCasesBlock, testCasesLink); + + MarkedBlock environmentBlock = requireMarkedBlock( + source, + ENVIRONMENT_START, + ENVIRONMENT_CONTENT, + ENVIRONMENT_END, + "environment/setup"); + return replaceBlock(source, environmentBlock, environmentLink); } - private static String replaceEnvironmentBody(String markdown, String replacementMarkdown) { + private static String requireReplacement(String replacementMarkdown, String sectionName) { String replacement = replacementMarkdown == null ? "" : replacementMarkdown.trim(); if (replacement.isBlank()) { - throw new IllegalArgumentException("Environment replacement is required."); - } - - Section section = findEnvironmentSection(markdown); - if (section == null) { - return markdown; - } - - Matcher heading = HEADING_PATTERN.matcher(section.content()); - if (!heading.find()) { - return markdown; + throw new IllegalArgumentException(sectionName + " replacement is required."); } - - StringBuilder result = new StringBuilder(markdown - .substring(0, section.start()) - .stripTrailing()) - .append("\n\n") - .append(heading.group().trim()) - .append("\n\n") - .append(replacement); - String suffix = markdown.substring(section.end()).stripLeading(); - if (!suffix.isBlank()) { - result.append("\n\n").append(suffix); - } - return result.toString().trim(); + return replacement; } - private static Section findTestCaseSection(String markdown) { - MarkedSection marked = findMarkedSection(markdown); - if (marked != null) { - int sectionEnd = marked.testContentEnd() == marked.markerEnd() - ? marked.markerEnd() + TEST_CASES_END.length() - : marked.testContentEnd(); - return new Section( - marked.markerStart(), - sectionEnd, - markdown.substring(marked.testContentStart(), marked.testContentEnd()).trim()); + private static MarkedBlock requireMarkedBlock( + String markdown, + String startMarker, + String contentMarker, + String endMarker, + String sectionName) { + MarkedBlock block = findMarkedBlock(markdown, startMarker, contentMarker, endMarker); + if (block == null) { + throw new IllegalArgumentException( + "A complete marked QA " + sectionName + " section is required."); } + return block; + } - Matcher legacy = LEGACY_SECTION_PATTERN.matcher(markdown); - if (!legacy.find()) { + private static MarkedBlock findMarkedBlock( + String markdown, + String startMarker, + String contentMarker, + String endMarker) { + if (countOccurrences(markdown, startMarker) != 1 + || countOccurrences(markdown, contentMarker) != 1 + || countOccurrences(markdown, endMarker) != 1) { return null; } - int headingLevel = legacy.group(1).length(); - int sectionEnd = markdown.length(); - Matcher headings = HEADING_PATTERN.matcher(markdown); - headings.region(legacy.end(), markdown.length()); - while (headings.find()) { - if (headings.group(1).length() <= headingLevel) { - sectionEnd = headings.start(); - break; - } - } - return new Section(legacy.start(), sectionEnd, markdown.substring(legacy.start(), sectionEnd).trim()); - } - /** - * Models occasionally put the closing marker after later peer sections. - * Treat the next heading at the test-section level (or higher) as the real - * boundary, while retaining the marker as the outer disclosure boundary. - */ - private static MarkedSection findMarkedSection(String markdown) { - int markerStart = markdown.indexOf(TEST_CASES_START); - if (markerStart < 0) { + int start = markdown.indexOf(startMarker); + int contentMarkerStart = markdown.indexOf(contentMarker, start + startMarker.length()); + int endMarkerStart = markdown.indexOf(endMarker, contentMarkerStart + contentMarker.length()); + if (start < 0 || contentMarkerStart < 0 || endMarkerStart < 0) { return null; } - int contentStart = markerStart + TEST_CASES_START.length(); - int markerEnd = markdown.indexOf(TEST_CASES_END, contentStart); - if (markerEnd < 0) { + + String heading = markdown.substring( + start + startMarker.length(), + contentMarkerStart).trim(); + String content = markdown.substring( + contentMarkerStart + contentMarker.length(), + endMarkerStart).trim(); + if (heading.isBlank() + || !heading.startsWith("#") + || heading.contains("\n") + || content.isBlank()) { return null; } - String markedContent = markdown.substring(contentStart, markerEnd); - Matcher testHeading = LEGACY_SECTION_PATTERN.matcher(markedContent); - if (!testHeading.find()) { - return new MarkedSection( - markerStart, - contentStart, - markerEnd, - contentStart, - markerEnd, - "### 3. Test Scenarios"); - } + return new MarkedBlock( + start, + endMarkerStart + endMarker.length(), + heading, + content); + } - int testContentStart = contentStart + testHeading.start(); - int testHeadingEnd = contentStart + testHeading.end(); - int testContentEnd = markerEnd; - int headingLevel = testHeading.group(1).length(); - - Matcher followingHeadings = HEADING_PATTERN.matcher(markdown); - followingHeadings.region(testHeadingEnd, markerEnd); - while (followingHeadings.find()) { - int followingLevel = followingHeadings.group(1).length(); - String followingTitle = followingHeadings.group(2).trim(); - if (followingLevel < headingLevel - || (followingLevel == headingLevel - && isLaterDocumentSection(followingTitle))) { - testContentEnd = followingHeadings.start(); - break; + private static int countOccurrences(String value, String marker) { + int count = 0; + int fromIndex = 0; + while (true) { + int found = value.indexOf(marker, fromIndex); + if (found < 0) { + return count; } + count++; + fromIndex = found + marker.length(); } - - return new MarkedSection( - markerStart, - contentStart, - markerEnd, - testContentStart, - testContentEnd, - testHeading.group().trim()); } - private static boolean isLaterDocumentSection(String headingTitle) { - return NUMBERED_SECTION_HEADING.matcher(headingTitle).matches() - || KNOWN_LATER_SECTION_HEADING.matcher(headingTitle).matches(); + private static String replaceBlock(String markdown, MarkedBlock block, String replacement) { + String prefix = markdown.substring(0, block.start()).stripTrailing(); + String suffix = markdown.substring(block.end()).stripLeading(); + StringBuilder result = new StringBuilder(); + appendDocumentPart(result, prefix); + appendDocumentPart(result, block.headingMarkdown() + "\n\n" + replacement); + appendDocumentPart(result, suffix); + return result.toString(); } - private static Section findEnvironmentSection(String markdown) { - Matcher headings = HEADING_PATTERN.matcher(markdown); - while (headings.find()) { - String title = headings.group(2).trim(); - if (!ENVIRONMENT_SECTION_HEADING.matcher(title).matches()) { + private static String removeBlocks(String markdown, List blocks) { + StringBuilder result = new StringBuilder(); + int cursor = 0; + for (MarkedBlock block : blocks) { + if (block.start() < cursor) { + cursor = Math.max(cursor, block.end()); continue; } - - int headingLevel = headings.group(1).length(); - int sectionStart = headings.start(); - int sectionEnd = findGeneratedFooterStart(markdown, headings.end()); - Matcher followingHeadings = HEADING_PATTERN.matcher(markdown); - followingHeadings.region(headings.end(), sectionEnd); - while (followingHeadings.find()) { - if (followingHeadings.group(1).length() <= headingLevel) { - sectionEnd = followingHeadings.start(); - break; - } - } - return new Section( - sectionStart, - sectionEnd, - markdown.substring(sectionStart, sectionEnd).trim()); + appendDocumentPart(result, markdown.substring(cursor, block.start())); + cursor = block.end(); } - return null; - } - - private static int findGeneratedFooterStart(String markdown, int fromIndex) { - Matcher footer = GENERATED_QA_FOOTER.matcher(markdown); - return footer.find(fromIndex) ? footer.start() : markdown.length(); + appendDocumentPart(result, markdown.substring(cursor)); + return result.toString(); } - private static String stripFirstHeading(String section) { - return HEADING_PATTERN.matcher(section).replaceFirst("").trim(); + private static void appendDocumentPart(StringBuilder target, String value) { + String part = value == null ? "" : value.trim(); + if (part.isBlank()) { + return; + } + if (!target.isEmpty()) { + target.append("\n\n"); + } + target.append(part); } private static String normalizeDocumentPart(String value) { @@ -308,12 +256,10 @@ private static List parseTestCases(String section) { if (!structured.isEmpty()) { return structured; } - - String fallback = stripSectionHeading(section); - if (fallback.isBlank()) { - return List.of(); - } - return List.of(new QaDocTestCase("Test scenarios", null, null, fallback)); + String fallback = normalizeDocumentPart(section); + return fallback.isBlank() + ? List.of() + : List.of(new QaDocTestCase("Test scenarios", null, null, fallback)); } private static List parseStructuredTestCases(String section) { @@ -360,28 +306,16 @@ private static String findFunctionalArea(String section, int beforePosition) { Matcher headings = HEADING_PATTERN.matcher(section); String latest = null; while (headings.find() && headings.start() < beforePosition) { - String candidate = headings.group(2).trim(); - if (!candidate.toLowerCase(Locale.ROOT).contains("test scenarios")) { - latest = candidate; - } + latest = headings.group(2).trim(); } return latest; } - private static String stripSectionHeading(String section) { - return LEGACY_SECTION_PATTERN.matcher(section).replaceFirst("").trim(); - } - - private record Section(int start, int end, String content) { - } - - private record MarkedSection( - int markerStart, - int contentStart, - int markerEnd, - int testContentStart, - int testContentEnd, - String heading + private record MarkedBlock( + int start, + int end, + String headingMarkdown, + String contentMarkdown ) { } diff --git a/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/model/project/config/RagConfigTest.java b/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/model/project/config/RagConfigTest.java index d769c9e3..f251be67 100644 --- a/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/model/project/config/RagConfigTest.java +++ b/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/model/project/config/RagConfigTest.java @@ -107,7 +107,7 @@ void shouldHaveDefaultBranchRetentionDaysConstant() { } @Test - void explicitIndexedBranchesShouldOverrideLegacyPushPatterns() { + void explicitIndexedBranchesAreTheOnlyRetainedNonPrimaryBranches() { RagConfig config = new RagConfig( true, "master", @@ -119,19 +119,18 @@ void explicitIndexedBranchesShouldOverrideLegacyPushPatterns() { true); assertThat(config.getEffectiveIndexedBranches()).containsExactly("develop", "support/1.x"); - assertThat(config.shouldHaveBranchIndex("develop", List.of("feature/**"))).isTrue(); - assertThat(config.shouldHaveBranchIndex("feature/one", List.of("feature/**"))).isFalse(); + assertThat(config.shouldHaveBranchIndex("develop")).isTrue(); + assertThat(config.shouldHaveBranchIndex("feature/one")).isFalse(); assertThat(config.isTransientBranchIndexesEnabled()).isTrue(); } @Test - void legacyConfigurationShouldContinueUsingPushPatterns() { + void configurationWithoutRetainedBranchesDoesNotInheritBranchAnalysisPatterns() { RagConfig config = new RagConfig(true, "master", null, null, true, 30); - assertThat(config.hasExplicitIndexedBranches()).isFalse(); - assertThat(config.shouldHaveBranchIndex("develop", List.of("develop", "support/**"))).isTrue(); - assertThat(config.shouldHaveBranchIndex("support/1.x", List.of("develop", "support/**"))).isTrue(); - assertThat(config.shouldHaveBranchIndex("feature/one", List.of("develop", "support/**"))).isFalse(); + assertThat(config.getEffectiveIndexedBranches()).isEmpty(); + assertThat(config.shouldHaveBranchIndex("develop")).isFalse(); + assertThat(config.shouldHaveBranchIndex("support/1.x")).isFalse(); assertThat(config.isTransientBranchIndexesEnabled()).isFalse(); } @@ -141,7 +140,7 @@ void transientIndexesRequireMultiBranchOwnership() { true, "master", null, null, false, 30, List.of("develop"), true); assertThat(config.isTransientBranchIndexesEnabled()).isFalse(); - assertThat(config.shouldHaveBranchIndex("develop", List.of("develop"))).isFalse(); + assertThat(config.shouldHaveBranchIndex("develop")).isFalse(); } @Test diff --git a/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParserTest.java b/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParserTest.java index 596e8bc9..84e1331b 100644 --- a/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParserTest.java +++ b/java-ecosystem/libs/core/src/test/java/org/rostilos/codecrow/core/service/qadoc/QaDocContentParserTest.java @@ -3,72 +3,89 @@ import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class QaDocContentParserTest { @Test - void separatesMarkedTestCasesFromOverviewAndPreservesStructuredDetails() { + void separatesExactSentinelBlocksWithoutInterpretingLocalizedHeadings() { String markdown = """ # QA Guide - ## What Changed - A user-visible flow changed. + ## Що змінилося + Змінився процес оформлення. - ## 3. Test Scenarios by Area + ## 3. Будь-яка локалізована назва + + ### Оформлення + **Успішне оформлення** (HIGH) + - **Передумови:** Товар у кошику + - **Кроки:** + 1. Підтвердити замовлення + - **Очікуваний результат:** Замовлення створено + - ### Checkout - **Complete checkout** (HIGH) - - **Preconditions:** A product is in the cart - - **Steps:** - 1. Submit the order - - **Expected Result:** The confirmation is shown + ## 4. Граничні випадки + Перевірити порожній кошик. - **Reject an empty address** (MEDIUM) - - **Expected Result:** A validation message is shown - + ## 5. Регресійні ризики + Перевірити збережений кошик. - ## Regression Risks - Verify saved carts. + + ## 6. Довільна назва підготовки + + - Використати тестовий платіжний профіль. + """; QaDocContent result = QaDocContentParser.parse(markdown); assertThat(result.overviewMarkdown()) - .contains("What Changed", "Regression Risks") - .doesNotContain("Complete checkout", "codecrow-test-cases"); - assertThat(result.environmentMarkdown()).isNull(); - assertThat(result.testCases()).hasSize(2); - assertThat(result.testCases().get(0)) + .contains("Що змінилося", "Граничні випадки", "Регресійні ризики") + .doesNotContain( + "Будь-яка локалізована назва", + "Успішне оформлення", + "Довільна назва підготовки", + "платіжний профіль", + "codecrow-"); + assertThat(result.testCases()).singleElement() .extracting(QaDocTestCase::title, QaDocTestCase::priority, QaDocTestCase::functionalArea) - .containsExactly("Complete checkout", "HIGH", "Checkout"); - assertThat(result.testCases().get(0).descriptionMarkdown()) - .contains("Preconditions", "Submit the order", "Expected Result"); + .containsExactly("Успішне оформлення", "HIGH", "Оформлення"); + assertThat(result.environmentMarkdown()) + .isEqualTo("- Використати тестовий платіжний профіль."); } @Test - void extractsLegacyEnglishTestScenarioSections() { + void doesNotInferShareableSectionsFromHeadingText() { QaDocContent result = QaDocContentParser.parse(""" - ### 1. Change Summary - Summary - ### 3. Test Scenarios + ## 3. Test Scenarios **Open settings** (LOW) - **Expected Result:** Settings appear - ### 4. Edge Cases - Try an empty state. + + ## 6. Environment and Setup Notes + Use staging. """); - assertThat(result.testCases()).singleElement() - .extracting(QaDocTestCase::title) - .isEqualTo("Open settings"); - assertThat(result.overviewMarkdown()).doesNotContain("Open settings"); + assertThat(result.testCases()).isEmpty(); + assertThat(result.environmentMarkdown()).isNull(); + assertThat(result.overviewMarkdown()) + .contains("Test Scenarios", "Open settings", "Environment and Setup Notes", "Use staging"); } @Test - void publicParsingRequiresMarkersAndAStructuredScenario() { + void publicParsingRequiresEveryExactTestCaseSentinelAndAStructuredScenario() { assertThat(QaDocContentParser.parseMarkedTestCases(""" - Secret overview text without a scenario + ### Test Scenarios + **Missing content sentinel** (HIGH) + + """)).isEmpty(); + assertThat(QaDocContentParser.parseMarkedTestCases(""" + + ### Test Scenarios + + General notes without a structured scenario. """)).isEmpty(); assertThat(QaDocContentParser.parseMarkedTestCases(""" @@ -78,8 +95,8 @@ void publicParsingRequiresMarkersAndAStructuredScenario() { } @Test - void replacesOnlyTheMarkedTestCaseBodyAndKeepsTheNumberedSection() { - String result = QaDocContentParser.replaceShareableSections(""" + void replacesOnlyExactBlocksWithLinksAndPreservesAllOtherSectionsAndFooter() { + String markdown = """ ### 1. Change Summary Checkout now supports gift cards. @@ -87,165 +104,176 @@ void replacesOnlyTheMarkedTestCaseBodyAndKeepsTheNumberedSection() { Web checkout only. - ### 3. Test Scenarios - - **Pay with a gift card** (HIGH) - - **Steps:** Enter a valid gift card. - - **Expected Result:** The balance is applied. + ### 3. Тестові сценарії + + ### Оформлення + **Оплатити подарунковою карткою** (HIGH) + - **Expected Result:** Баланс застосовано. - ### 4. Edge Cases - Test an expired gift card. - """, - "https://codecrow.cloud/share#token=ccs_public-token&tab=test-cases", - "https://codecrow.cloud/share#token=ccs_public-token&tab=environment"); - - assertThat(result) - .contains("### 1. Change Summary", "Checkout now supports gift cards") - .contains("### 2. Scope", "Web checkout only") - .contains("### 3. Test Scenarios\n\nhttps://codecrow.cloud/share#token=ccs_public-token") - .contains("### 4. Edge Cases", "Test an expired gift card") - .doesNotContain("Pay with a gift card", "The balance is applied"); - } - - @Test - void preservesLaterPeerSectionsWhenTheModelPlacesTheEndMarkerTooLate() { - String markdown = """ - ### 1. Change Summary - Checkout changed. - - - ### 3. Test Scenarios - ### Checkout - **Pay with a gift card** (HIGH) - - **Expected Result:** The balance is applied. - ### 4. Edge Cases and Negative Testing - - Try an expired gift card. + Test an expired gift card. ### 5. Regression Risks - - Existing card payments. + Verify saved cards. - ### 6. Environment and Setup Notes - - Use the QA payment environment. - + + ### 6. Налаштування середовища + + Use the QA payment environment. + + + --- + *🐦 Generated by [CodeCrow](https://codecrow.app) QA Auto-Documentation* + """; - QaDocContent parsed = QaDocContentParser.parse(markdown); - String comment = QaDocContentParser.replaceShareableSections( + String result = QaDocContentParser.replaceShareableSections( markdown, "https://codecrow.cloud/share#token=ccs_public-token&tab=test-cases", "https://codecrow.cloud/share#token=ccs_public-token&tab=environment"); - assertThat(parsed.testCases()).singleElement() - .extracting(QaDocTestCase::title, QaDocTestCase::functionalArea) - .containsExactly("Pay with a gift card", "Checkout"); - assertThat(parsed.overviewMarkdown()) - .contains( - "### 4. Edge Cases and Negative Testing", - "### 5. Regression Risks") - .doesNotContain( - "Pay with a gift card", - "codecrow-test-cases", - "### 6. Environment and Setup Notes", - "Use the QA payment environment"); - assertThat(parsed.environmentMarkdown()) - .isEqualTo("- Use the QA payment environment."); - assertThat(comment) - .contains("### 3. Test Scenarios\n\nhttps://codecrow.cloud/share#token=ccs_public-token&tab=test-cases") - .contains( - "### 4. Edge Cases and Negative Testing", - "Try an expired gift card", - "### 5. Regression Risks", - "Existing card payments", - "### 6. Environment and Setup Notes", - "https://codecrow.cloud/share#token=ccs_public-token&tab=environment") + assertThat(result) + .contains("### 1. Change Summary", "Checkout now supports gift cards") + .contains("### 2. Scope", "Web checkout only") + .contains("### 3. Тестові сценарії\n\n" + + "https://codecrow.cloud/share#token=ccs_public-token&tab=test-cases") + .contains("### 4. Edge Cases and Negative Testing", "Test an expired gift card") + .contains("### 5. Regression Risks", "Verify saved cards") + .contains("### 6. Налаштування середовища\n\n" + + "https://codecrow.cloud/share#token=ccs_public-token&tab=environment") + .contains("Generated by [CodeCrow]", "") .doesNotContain( - "Pay with a gift card", - "The balance is applied", - "Use the QA payment environment") - .contains("tab=test-cases\n\n\n### 4."); + "Оплатити подарунковою карткою", + "Баланс застосовано", + "Use the QA payment environment", + "codecrow-test-cases:", + "codecrow-environment:"); } @Test - void replacesEnvironmentBodyAndPreservesTheGeneratedFooter() { - String markdown = """ - ### 1. Change Summary - Checkout changed. + void parseStripsGeneratedFooterButLeavesMiddleSectionsUntouched() { + QaDocContent parsed = QaDocContentParser.parse(""" + ### 1. Summary + Summary. - ### 3. Test Scenarios - **Pay successfully** (HIGH) - - **Expected Result:** Payment succeeds. + ### 3. Tests + + **Works** (HIGH) + - **Expected Result:** It works. ### 4. Edge Cases - Try an expired card. + Edge content. ### 5. Regression Risks - Verify saved cards. + Regression content. - ### 6. Environment and Setup Notes - Use the QA payment environment. + + ### 6. Setup + + No special setup is required. + --- *🐦 Generated by [CodeCrow](https://codecrow.app) QA Auto-Documentation* - """; - QaDocContent parsed = QaDocContentParser.parse(markdown); - String comment = QaDocContentParser.replaceShareableSections( - markdown, - "https://codecrow.cloud/share#token=ccs_public-token&tab=test-cases", - "https://codecrow.cloud/share#token=ccs_public-token&tab=environment"); + """); assertThat(parsed.overviewMarkdown()) - .contains("Change Summary", "Edge Cases", "Regression Risks") - .doesNotContain("Generated by", "codecrow-qa-autodoc"); - assertThat(parsed.environmentMarkdown()).isEqualTo("Use the QA payment environment."); - assertThat(comment) - .contains("### 4. Edge Cases", "Try an expired card") - .contains("### 5. Regression Risks", "Verify saved cards") - .contains("### 6. Environment and Setup Notes\n\n" - + "https://codecrow.cloud/share#token=ccs_public-token&tab=environment") - .contains("Generated by [CodeCrow]", "") - .doesNotContain("Use the QA payment environment."); + .contains("Summary", "Edge Cases", "Edge content", "Regression Risks", "Regression content") + .doesNotContain("Generated by", "codecrow-qa-autodoc", "### 3. Tests", "### 6. Setup"); + assertThat(parsed.environmentMarkdown()).isEqualTo("No special setup is required."); } @Test - void separatesSetupAndEnvironmentNotesFromAnUnmarkedDocument() { - QaDocContent parsed = QaDocContentParser.parse(""" - # QA Testing Guide — Checkout + void rejectsJiraReplacementWhenEitherSentinelBlockIsIncomplete() { + String missingEnvironmentBlock = """ + + ### 3. Tests + + **Works** (HIGH) + - **Expected Result:** It works. + - ## 1. What Changed - Saved cards can now be selected at checkout. + ### 6. Environment and Setup Notes + Private environment content must never be posted. + """; - ## 6. Setup and Environment Notes - - Enable the saved-card feature flag. - - Use a customer with an existing payment method. - """); + assertThatThrownBy(() -> QaDocContentParser.replaceShareableSections( + missingEnvironmentBlock, + "test-link", + "environment-link")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("environment/setup"); + } - assertThat(parsed.overviewMarkdown()) - .contains("What Changed", "Saved cards") - .doesNotContain("Setup and Environment Notes", "feature flag"); - assertThat(parsed.environmentMarkdown()) - .contains("Enable the saved-card feature flag", "existing payment method") - .doesNotContain("## 6."); + @Test + void rejectsDuplicateSentinelsInsteadOfGuessingWhichBlockToUse() { + String duplicatedTestStart = """ + + + ### 3. Tests + + **Works** (HIGH) + - **Expected Result:** It works. + + + + ### 6. Setup + + No special setup. + + """; + + assertThat(QaDocContentParser.hasCompleteShareableSections(duplicatedTestStart)).isFalse(); + assertThat(QaDocContentParser.parseMarkedTestCases(duplicatedTestStart)).isEmpty(); } @Test - void preservesSectionsAfterEnvironmentNotesInTheOverview() { - QaDocContent parsed = QaDocContentParser.parse(""" - ## Overview - Summary. + void rejectsBodyTextPlacedInTheHeadingSlot() { + String invalid = """ + + ### 3. Tests + This must not leak into Jira before the link. + + **Works** (HIGH) + - ## Environment and Setup Notes - Use staging. + + ### 6. Setup + + No special setup. + + """; - ## Appendix - Contact the QA lead. - """); + assertThat(QaDocContentParser.hasCompleteShareableSections(invalid)).isFalse(); + assertThatThrownBy(() -> QaDocContentParser.replaceShareableSections( + invalid, + "test-link", + "environment-link")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("test-case"); + } + + @Test + void recognizesACompleteShareableDocumentOnlyWhenBothBlocksAreValid() { + String complete = """ + + ### 3. Tests + + **Works** (HIGH) + - **Expected Result:** It works. + + + + ### 6. Setup + + No special setup. + + """; - assertThat(parsed.environmentMarkdown()).isEqualTo("Use staging."); - assertThat(parsed.overviewMarkdown()).contains("Overview", "Appendix", "Contact the QA lead"); + assertThat(QaDocContentParser.hasCompleteShareableSections(complete)).isTrue(); } } diff --git a/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceService.java b/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceService.java index f6665d54..42361f43 100644 --- a/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceService.java +++ b/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceService.java @@ -188,17 +188,28 @@ private void rebuildOne(Project project, BranchBuildPlan plan, Consumer includePatterns = config.includePatterns() != null + ? config.includePatterns() : List.of(); + List excludePatterns = config.excludePatterns() != null + ? config.excludePatterns() : List.of(); if (config.includePatterns() == null || config.excludePatterns() == null) { - throw new IllegalStateException("RAG include/exclude patterns are unavailable"); + log.info( + "Optional RAG path patterns are unset; using empty filters: project={}, branch={}", + project.getId(), branch); + emitEvent(events, Map.of( + "type", "progress", + "stage", "branch_config", + "branch", branch, + "message", "No custom RAG path filters are configured; indexing all supported files")); } admittedBuild = buildAdmissionService.admit( project, - branch, - revision, - kind, - JobTriggerSource.UI, - lock.get(), - BranchIndexBuildAdmissionService.BuildOrigin.OPERATOR); + branch, + revision, + kind, + JobTriggerSource.UI, + lock.get(), + BranchIndexBuildAdmissionService.BuildOrigin.OPERATOR); job = admittedBuild.job(); Job admittedJob = job; jobService.logToJob( @@ -216,8 +227,8 @@ private void rebuildOne(Project project, BranchBuildPlan plan, Consumer { Map forwarded = new LinkedHashMap<>(event); forwarded.put("type", "progress"); diff --git a/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImpl.java b/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImpl.java index 6cad6d3f..cd56b8cb 100644 --- a/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImpl.java +++ b/java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImpl.java @@ -324,13 +324,29 @@ && isRagEnabled(project) Set modifiedFiles = Set.of(); Set deletedFiles = Set.of(); int addedOrModifiedSize = 0; + boolean checkpointOnlyAdvance = false; + boolean unrecognizedLegacyDiff = false; - // Preserve the legacy checkpoint flow, including its no-op return - // before a job or lock is created. Exact generations are resolved - // below only after this update owns the branch RAG lock. + // Legacy collections reconcile from their own completed checkpoint. + // A same-revision request is a true no-op. A newer revision with an + // empty range must still own a durable job and advance its checkpoint + // so later pushes do not repeatedly start from stale state. if (!exactGenerationMode) { - String effectiveRawDiff = resolveDiffFromCompletedCheckpoint( + LegacyDiffResolution diffResolution = resolveDiffFromCompletedCheckpoint( project, branchName, commitHash, rawDiff); + if (diffResolution.alreadyCurrent()) { + String message = String.format( + "RAG index for branch '%s' already represents commit %s", + branchName, commitHash); + log.info(message); + emitEvent(eventConsumer, Map.of( + "type", "info", + "state", "rag_current", + "message", message)); + return true; + } + + String effectiveRawDiff = diffResolution.rawDiff(); log.info("RAG checkpoint reconciliation complete; parsing effective diff..."); IncrementalRagUpdateService.DiffResult diffResult = incrementalRagUpdateService.parseDiffForRag(effectiveRawDiff); @@ -342,11 +358,16 @@ && isRagEnabled(project) log.info("Diff parsed: added={}, modified={}, deleted={}", addedFiles, modifiedFiles, deletedFiles); if (addedOrModifiedSize == 0 && deletedFiles.isEmpty()) { - log.info("Skipping RAG incremental update - no files changed in diff"); - return true; + checkpointOnlyAdvance = effectiveRawDiff == null + || effectiveRawDiff.isBlank(); + unrecognizedLegacyDiff = !checkpointOnlyAdvance; + log.info(checkpointOnlyAdvance + ? "No files changed; a durable RAG job will advance the checkpoint" + : "Non-empty RAG diff did not contain recognizable file changes"); + } else { + log.info("RAG incremental update: {} files to add/update, {} files to delete", + addedOrModifiedSize, deletedFiles.size()); } - log.info("RAG incremental update: {} files to add/update, {} files to delete", - addedOrModifiedSize, deletedFiles.size()); } if (!exactGenerationMode) { @@ -357,6 +378,10 @@ && isRagEnabled(project) String.format( "Starting incremental RAG update for branch '%s' (commit: %s) - %d files to update, %d to delete", branchName, commitHash, addedOrModifiedSize, deletedFiles.size())); + if (unrecognizedLegacyDiff) { + throw new IOException( + "RAG checkpoint diff was non-empty but contained no recognizable file changes"); + } } Optional ragLockKey = analysisLockService.acquireLock( @@ -599,13 +624,20 @@ && isRagEnabled(project) } } - String completionMessage = exactGenerationMode - ? String.format( - "Exact RAG snapshot activated: %d documents, %d chunks", - documentCount, chunkCount != null ? chunkCount : 0) - : String.format( - "RAG index updated: %d files updated, %d deleted, %d non-text files skipped", - filesUpdated, filesDeleted, filesSkipped); + String completionMessage; + if (exactGenerationMode) { + completionMessage = String.format( + "Exact RAG snapshot activated: %d documents, %d chunks", + documentCount, chunkCount != null ? chunkCount : 0); + } else if (checkpointOnlyAdvance) { + completionMessage = String.format( + "RAG checkpoint advanced for branch '%s' to commit %s; no files changed", + branchName, commitHash); + } else { + completionMessage = String.format( + "RAG index updated: %d files updated, %d deleted, %d non-text files skipped", + filesUpdated, filesDeleted, filesSkipped); + } emitEvent(eventConsumer, Map.of( "type", "status", "state", "rag_complete", @@ -759,7 +791,7 @@ private LegacyRagOwnershipLostException(String message) { * The caller's branch-analysis diff is used only when no completed * checkpoint exists yet for this branch. */ - private String resolveDiffFromCompletedCheckpoint( + private LegacyDiffResolution resolveDiffFromCompletedCheckpoint( Project project, String branchName, String commitHash, @@ -785,11 +817,11 @@ private String resolveDiffFromCompletedCheckpoint( if (checkpoint == null) { log.info("No completed RAG checkpoint for branch {}; using supplied initial diff", branchName); - return suppliedDiff; + return new LegacyDiffResolution(suppliedDiff, false); } if (checkpoint.equals(commitHash)) { log.info("RAG checkpoint already represents branch {} commit {}", branchName, commitHash); - return ""; + return new LegacyDiffResolution("", true); } VcsRepoBinding vcsRepoBinding = project.getVcsRepoBinding(); @@ -805,7 +837,10 @@ private String resolveDiffFromCompletedCheckpoint( branchName, checkpoint, commitHash); String catchUpDiff = vcsClient.getBranchDiff( workspaceSlug, repoSlug, checkpoint, commitHash); - return catchUpDiff != null ? catchUpDiff : ""; + return new LegacyDiffResolution(catchUpDiff != null ? catchUpDiff : "", false); + } + + private record LegacyDiffResolution(String rawDiff, boolean alreadyCurrent) { } // ========================================================================== @@ -870,8 +905,21 @@ public void createOrUpdateBranchIndex( String branchCommit, String rawDiff, Consumer> eventConsumer) { - // With single-collection architecture, we just do incremental update - // No separate collection needed - branch data goes into shared collection + if (!isRagEnabled(project)) { + return; + } + if (!branchName.equals(getBaseBranch(project)) + && !shouldHaveBranchIndex(project, branchName)) { + log.info("Skipping branch index mutation for non-retained branch: project={}, branch={}", + project.getId(), branchName); + emitEvent(eventConsumer, Map.of( + "type", "info", + "state", "rag_skipped", + "message", "Branch is not configured as a retained RAG branch")); + return; + } + // Dispatch only after this branch-push compatibility entry point has + // enforced durable ownership. The trigger selects legacy or exact mode. triggerIncrementalUpdate(project, branchName, branchCommit, rawDiff, eventConsumer); } @@ -1487,24 +1535,17 @@ private boolean ensureMainIndexUpToDate( log.info("Main RAG index outdated for project={}: indexed={}, current={}", project.getId(), indexedCommit, currentCommit); - // Fetch diff between indexed commit and current commit - String rawDiff = vcsClient.getBranchDiff(workspaceSlug, repoSlug, indexedCommit, currentCommit); - - if (rawDiff == null || rawDiff.isEmpty()) { - log.debug("No diff between {} and {} - index is up to date", indexedCommit, currentCommit); - ragIndexTrackingService.markUpdatingCompleted(project, branchName, currentCommit, 0, 0, null); - return true; - } - emitEvent(eventConsumer, Map.of( "type", "status", "state", "rag_update", "message", String.format("Updating RAG index from %s to %s", indexedCommit.substring(0, 7), currentCommit.substring(0, 7)))); - // Trigger incremental update + // The locked trigger owns the checkpoint-to-target compare, durable + // child job, and terminal checkpoint transition. In particular, an + // empty compare must not update the checkpoint directly here. return triggerIncrementalUpdate( - project, branchName, currentCommit, rawDiff, eventConsumer); + project, branchName, currentCommit, "", eventConsumer); } /** @@ -1575,32 +1616,18 @@ private boolean ensureBranchIndexUpToDate( log.info("Branch index outdated for project={}, branch={}: indexed={}, current={} - fetching incremental diff", project.getId(), targetBranch, indexedCommit, currentCommit); - // Fetch diff between last indexed commit and current commit (incremental) - String rawDiff = vcsClient.getBranchDiff(workspaceSlug, repoSlug, indexedCommit, currentCommit); - log.info("Incremental diff for branch '{}' ({} -> {}): bytes={}", - targetBranch, indexedCommit.substring(0, 7), currentCommit.substring(0, 7), - rawDiff != null ? rawDiff.length() : 0); - - if (rawDiff == null || rawDiff.isEmpty()) { - log.info("No diff between {} and {} - updating commit hash only", indexedCommit, currentCommit); - // Update commit hash - branchIndex.setCommitHash(currentCommit); - branchIndex.setUpdatedAt(OffsetDateTime.now()); - ragBranchIndexRepository.save(branchIndex); - return true; - } - emitEvent(eventConsumer, Map.of( "type", "status", "state", "branch_update", "message", - String.format("Updating branch %s index (incremental: %d bytes)", targetBranch, rawDiff.length()))); + String.format("Reconciling branch %s index from %s to %s", + targetBranch, indexedCommit, currentCommit))); - // Trigger incremental update for this branch - log.info("Triggering incremental branch update for '{}' with {} bytes diff", - targetBranch, rawDiff.length()); + // The trigger reacquires the complete range from this branch's durable + // checkpoint and owns both the job and checkpoint transition. + log.info("Triggering incremental branch reconciliation for '{}'", targetBranch); return triggerIncrementalUpdate( - project, targetBranch, currentCommit, rawDiff, eventConsumer); + project, targetBranch, currentCommit, "", eventConsumer); } /** diff --git a/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceServiceTest.java b/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceServiceTest.java index 512d545a..e73930a9 100644 --- a/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceServiceTest.java +++ b/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/branch/BranchIndexMaintenanceServiceTest.java @@ -26,6 +26,71 @@ class BranchIndexMaintenanceServiceTest { + @Test + void unsetOptionalPatternsUseEmptyFiltersForInitialSnapshot() throws Exception { + RagOperationsService ragOperations = mock(RagOperationsService.class); + VcsClientProvider vcsClients = mock(VcsClientProvider.class); + BranchIndexGenerationBuildService builds = mock( + BranchIndexGenerationBuildService.class); + BranchIndexBuildAdmissionService admissions = mock( + BranchIndexBuildAdmissionService.class); + RagIndexTrackingService tracking = mock(RagIndexTrackingService.class); + AnalysisLockService locks = mock(AnalysisLockService.class); + AnalysisJobService jobs = mock(AnalysisJobService.class); + BranchIndexMaintenanceService service = new BranchIndexMaintenanceService( + ragOperations, vcsClients, builds, admissions, tracking, locks, jobs, + Runnable::run, 1); + + Project project = mock(Project.class); + when(project.getId()).thenReturn(42L); + when(project.getConfiguration()).thenReturn(new ProjectConfig( + false, "main", null, new RagConfig(true, "main"))); + VcsRepoBinding binding = mock(VcsRepoBinding.class); + VcsConnection connection = new VcsConnection(); + when(project.getVcsRepoBinding()).thenReturn(binding); + when(binding.getVcsConnection()).thenReturn(connection); + when(binding.getExternalNamespace()).thenReturn("workspace"); + when(binding.getExternalRepoSlug()).thenReturn("repository"); + VcsClient vcs = mock(VcsClient.class); + when(vcsClients.getClient(connection)).thenReturn(vcs); + when(vcs.getLatestCommitHash("workspace", "repository", "main")) + .thenReturn("revision-a"); + when(ragOperations.getBaseBranch(project)).thenReturn("main"); + when(locks.acquireLock(eq(project), eq("main"), any(), eq("revision-a"), isNull())) + .thenReturn(Optional.of("rag-lock")); + + Job job = mock(Job.class); + when(job.getId()).thenReturn(91L); + var prepared = new BranchIndexGenerationBuildService.PreparedBuild( + 81L, "physical-generation", false, null, "rag-lock"); + when(admissions.admit( + eq(project), eq("main"), eq("revision-a"), + eq(RagBranchIndexKind.PRIMARY), any(), eq("rag-lock"), any())) + .thenReturn(new BranchIndexBuildAdmissionService.AdmittedBuild( + job, prepared, + BranchIndexBuildAdmissionService.ProjectStatusAdmission.INDEXING)); + when(builds.execute( + eq(project), eq(connection), eq("workspace"), eq("repository"), + eq("main"), eq("revision-a"), eq(RagBranchIndexKind.PRIMARY), + eq(List.of()), eq(List.of()), eq(prepared), any())) + .thenReturn(Map.of("document_count", 12, "chunk_count", 34)); + + Map outcome = service.rebuild( + project, "main", false, ignored -> { }); + + assertThat(outcome.get("branches")).isEqualTo(List.of("main")); + assertThat(outcome.get("failedBranches")).isEqualTo(Map.of()); + verify(builds).execute( + eq(project), eq(connection), eq("workspace"), eq("repository"), + eq("main"), eq("revision-a"), eq(RagBranchIndexKind.PRIMARY), + eq(List.of()), eq(List.of()), eq(prepared), any()); + verify(tracking).reconcilePublishedGeneration( + project, "main", "revision-a", 12, 34, 91L); + verify(jobs).completeJob(job, Map.of("branch", "main", "revision", "revision-a")); + verify(jobs, never()).failJob(any(), anyString()); + verify(locks).releaseLock("rag-lock"); + } + @Test void observerAndLockCleanupFailuresCannotReverseAPublishedBuild() throws Exception { RagOperationsService ragOperations = mock(RagOperationsService.class); diff --git a/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImplTest.java b/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImplTest.java index f130d89e..14de32f0 100644 --- a/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImplTest.java +++ b/java-ecosystem/libs/rag-engine/src/test/java/org/rostilos/codecrow/ragengine/service/RagOperationsServiceImplTest.java @@ -16,6 +16,7 @@ import org.rostilos.codecrow.core.model.job.Job; import org.rostilos.codecrow.core.model.job.JobTriggerSource; import org.rostilos.codecrow.core.model.project.Project; +import org.rostilos.codecrow.core.model.project.config.BranchAnalysisConfig; import org.rostilos.codecrow.core.model.project.config.ProjectConfig; import org.rostilos.codecrow.core.model.project.config.RagConfig; import org.rostilos.codecrow.core.model.rag.RagBranchIndex; @@ -773,6 +774,31 @@ void testCreateOrUpdateBranchIndex_WhenNotEnabled() { verifyNoInteractions(analysisJobService); } + @Test + void createOrUpdateBranchIndexRejectsBranchAnalysisPatternWithoutRetention() { + ReflectionTestUtils.setField(service, "ragApiEnabled", true); + testProject.setConfiguration(new ProjectConfig( + false, + "main", + new BranchAnalysisConfig(List.of("main"), List.of("release/**")), + new RagConfig(true, "main", null, null, true, 30, null, false))); + service = spy(service); + @SuppressWarnings("unchecked") + Consumer> eventConsumer = mock(Consumer.class); + + service.createOrUpdateBranchIndex( + testProject, + "release/preview", + "main", + "release-commit", + "diff", + eventConsumer); + + verify(service, never()).triggerIncrementalUpdate(any(), any(), any(), any(), any()); + verify(eventConsumer).accept(argThat(event -> + "rag_skipped".equals(event.get("state")))); + } + @Test void testEnsureRagIndexUpToDate_WhenNotEnabled() { ReflectionTestUtils.setField(service, "ragApiEnabled", false); @@ -1334,21 +1360,98 @@ void legacyOwnershipIsReconfirmedBeforeCheckpointAndJobCompletion() } @Test - void testTriggerIncrementalUpdate_EmptyDiff_NoFilesChanged() { + void newerCommitWithEmptyRangeCreatesJobAndAdvancesCheckpoint() throws Exception { setupRagEnabled(); + setupVcsBinding(); @SuppressWarnings("unchecked") Consumer> eventConsumer = mock(Consumer.class); + Job job = mock(Job.class); + when(job.getId()).thenReturn(82L); when(incrementalRagUpdateService.shouldPerformIncrementalUpdate(testProject)).thenReturn(true); - when(incrementalRagUpdateService.parseDiffForRag("empty")) - .thenReturn(new IncrementalRagUpdateService.DiffResult(java.util.Collections.emptySet(), - java.util.Collections.emptySet(), java.util.Collections.emptySet())); + when(incrementalRagUpdateService.parseDiffForRag("")) + .thenReturn(new IncrementalRagUpdateService.DiffResult( + Set.of(), Set.of(), Set.of())); + when(analysisJobService.createRagIndexJob( + testProject, false, JobTriggerSource.WEBHOOK, "main", "abc")) + .thenReturn(job); + when(analysisLockService.acquireLock( + any(), eq("main"), any(), eq("abc"), isNull())) + .thenReturn(Optional.of("lock-key")); + when(incrementalRagUpdateService.performIncrementalUpdate( + any(), any(), anyString(), anyString(), anyString(), anyString(), + anySet(), anySet(), anySet())) + .thenReturn(Map.of("status", "completed")); boolean result = - service.triggerIncrementalUpdate(testProject, "main", "abc", "empty", eventConsumer); + service.triggerIncrementalUpdate(testProject, "main", "abc", "", eventConsumer); assertThat(result).isTrue(); - verifyNoInteractions(analysisLockService); + verify(analysisJobService).startJob(job); + verify(analysisLockService).acquireLock( + any(), eq("main"), any(), eq("abc"), isNull()); + verify(ragIndexTrackingService).markUpdatingStarted( + testProject, "main", "abc", 82L); + verify(legacyRagUpdateCompletionService).complete( + eq(testProject), eq("main"), eq("abc"), eq(82L), + any(), eq(true), eq(0), eq(0), isNull(), eq(Set.of())); + verify(analysisJobService).recordExternallyCompletedJob( + eq(job), eq("rag_complete"), contains("checkpoint advanced")); + verify(eventConsumer).accept(argThat(event -> + "rag_complete".equals(event.get("state")) + && String.valueOf(event.get("message")) + .contains("checkpoint advanced"))); + } + + @Test + void alreadyCurrentLegacyCommitDoesNotCreateAJobOrClaimAnUpdate() { + setupRagEnabled(); + RagIndexStatus completedStatus = new RagIndexStatus(); + completedStatus.setIndexedCommitHash("abc"); + when(incrementalRagUpdateService.shouldPerformIncrementalUpdate(testProject)) + .thenReturn(true); + when(ragIndexTrackingService.getIndexStatus(testProject)) + .thenReturn(Optional.of(completedStatus)); + @SuppressWarnings("unchecked") + Consumer> eventConsumer = mock(Consumer.class); + + boolean result = service.triggerIncrementalUpdate( + testProject, "main", "abc", "stale caller diff", eventConsumer); + + assertThat(result).isTrue(); + verifyNoInteractions(analysisJobService, analysisLockService, vcsClientProvider); + verify(incrementalRagUpdateService, never()).parseDiffForRag(anyString()); + verify(eventConsumer).accept(argThat(event -> + "rag_current".equals(event.get("state")) + && String.valueOf(event.get("message")) + .contains("already represents"))); + } + + @Test + void nonEmptyUnrecognizedDiffCreatesFailedJobAndRetainsCheckpoint() { + setupRagEnabled(); + Job job = mock(Job.class); + when(incrementalRagUpdateService.shouldPerformIncrementalUpdate(testProject)) + .thenReturn(true); + when(incrementalRagUpdateService.parseDiffForRag("provider payload")) + .thenReturn(new IncrementalRagUpdateService.DiffResult( + Set.of(), Set.of(), Set.of())); + when(analysisJobService.createRagIndexJob( + testProject, false, JobTriggerSource.WEBHOOK, "main", "abc")) + .thenReturn(job); + @SuppressWarnings("unchecked") + Consumer> eventConsumer = mock(Consumer.class); + + boolean result = service.triggerIncrementalUpdate( + testProject, "main", "abc", "provider payload", eventConsumer); + + assertThat(result).isFalse(); + verify(analysisJobService).startJob(job); + verify(analysisJobService).failJob( + eq(job), contains("no recognizable file changes")); + verifyNoInteractions(analysisLockService, legacyRagUpdateCompletionService); + verify(eventConsumer).accept(argThat(event -> + "rag_error".equals(event.get("state")))); } @Test @@ -1603,6 +1706,31 @@ void updateBranchIndexRejectsBranchOutsideRetainedConfiguration() { verify(eventConsumer).accept(argThat(event -> "rag_skipped".equals(event.get("state")))); } + @Test + void branchPushPatternDoesNotAuthorizeRetainedRagIndexUpdate() { + ReflectionTestUtils.setField(service, "ragApiEnabled", true); + RagConfig ragConfig = new RagConfig( + true, "main", null, null, true, 30, null, false); + ProjectConfig config = new ProjectConfig( + false, + "main", + new BranchAnalysisConfig(List.of("main"), List.of("release/**")), + ragConfig); + testProject.setConfiguration(config); + when(ragIndexTrackingService.isProjectIndexed(testProject)).thenReturn(true); + @SuppressWarnings("unchecked") + Consumer> eventConsumer = mock(Consumer.class); + + boolean result = service.updateBranchIndex( + testProject, "release/preview", eventConsumer); + + assertThat(result).isFalse(); + verifyNoInteractions(vcsClientProvider); + verify(eventConsumer).accept(argThat(event -> + "rag_skipped".equals(event.get("state")) + && event.get("message").toString().contains("not configured"))); + } + @Test void updateBranchIndexEmptyDiffSeedsExactSnapshot() throws Exception { RagBranchIndexRegistryService registry = mock(RagBranchIndexRegistryService.class); @@ -1723,6 +1851,7 @@ void testEnsureRagIndexUpToDate_MainBranch_UpToDate() throws Exception { @Test void testEnsureRagIndexUpToDate_MainBranch_Outdated() throws Exception { + service = spy(service); setupRagEnabled(); setupVcsBinding(); when(ragIndexTrackingService.isProjectIndexed(testProject)).thenReturn(true); @@ -1732,19 +1861,22 @@ void testEnsureRagIndexUpToDate_MainBranch_Outdated() throws Exception { RagIndexStatus status = mock(RagIndexStatus.class); when(status.getIndexedCommitHash()).thenReturn("old-commit"); when(ragIndexTrackingService.getIndexStatus(testProject)).thenReturn(Optional.of(status)); - when(mockVcs.getBranchDiff("my-workspace", "my-repo", "old-commit", "new-commit")).thenReturn("diff"); - // triggerIncrementalUpdate is called internally but shouldPerform returns false - when(incrementalRagUpdateService.shouldPerformIncrementalUpdate(testProject)).thenReturn(false); @SuppressWarnings("unchecked") Consumer> eventConsumer = mock(Consumer.class); + doReturn(true).when(service).triggerIncrementalUpdate( + testProject, "main", "new-commit", "", eventConsumer); - service.ensureRagIndexUpToDate(testProject, "main", eventConsumer); + boolean result = service.ensureRagIndexUpToDate(testProject, "main", eventConsumer); - verify(mockVcs).getBranchDiff("my-workspace", "my-repo", "old-commit", "new-commit"); + assertThat(result).isTrue(); + verify(service).triggerIncrementalUpdate( + testProject, "main", "new-commit", "", eventConsumer); + verify(mockVcs, never()).getBranchDiff(anyString(), anyString(), anyString(), anyString()); } @Test - void testEnsureRagIndexUpToDate_MainBranch_NullDiff() throws Exception { + void outdatedMainCheckpointCannotBeAdvancedOutsideDurableTrigger() throws Exception { + service = spy(service); setupRagEnabled(); setupVcsBinding(); when(ragIndexTrackingService.isProjectIndexed(testProject)).thenReturn(true); @@ -1754,14 +1886,19 @@ void testEnsureRagIndexUpToDate_MainBranch_NullDiff() throws Exception { RagIndexStatus status = mock(RagIndexStatus.class); when(status.getIndexedCommitHash()).thenReturn("old-commit"); when(ragIndexTrackingService.getIndexStatus(testProject)).thenReturn(Optional.of(status)); - when(mockVcs.getBranchDiff("my-workspace", "my-repo", "old-commit", "new-commit")).thenReturn(null); @SuppressWarnings("unchecked") Consumer> eventConsumer = mock(Consumer.class); + doReturn(false).when(service).triggerIncrementalUpdate( + testProject, "main", "new-commit", "", eventConsumer); boolean result = service.ensureRagIndexUpToDate(testProject, "main", eventConsumer); - assertThat(result).isTrue(); - verify(ragIndexTrackingService).markUpdatingCompleted(testProject, "main", "new-commit", 0, 0, null); + assertThat(result).isFalse(); + verify(service).triggerIncrementalUpdate( + testProject, "main", "new-commit", "", eventConsumer); + verify(ragIndexTrackingService, never()).markUpdatingCompleted( + any(), anyString(), anyString(), any(), any(), any()); + verify(mockVcs, never()).getBranchDiff(anyString(), anyString(), anyString(), anyString()); } @Test @@ -1858,6 +1995,7 @@ void testEnsureRagIndexUpToDate_DifferentBranch_UpToDate() throws Exception { @Test void testEnsureRagIndexUpToDate_DifferentBranch_Outdated() throws Exception { + service = spy(service); setupRagEnabled(); setupVcsBinding(); when(ragIndexTrackingService.isProjectIndexed(testProject)).thenReturn(true); @@ -1874,20 +2012,22 @@ void testEnsureRagIndexUpToDate_DifferentBranch_Outdated() throws Exception { branchIndex.setCommitHash("f1"); when(ragBranchIndexRepository.findByProjectIdAndBranchName(100L, "feature")) .thenReturn(Optional.of(branchIndex)); - when(mockVcs.getBranchDiff("my-workspace", "my-repo", "f1", "f2")).thenReturn("incremental diff"); - // triggerIncrementalUpdate called internally - shouldPerform returns false so - // it exits early - lenient().when(incrementalRagUpdateService.shouldPerformIncrementalUpdate(testProject)).thenReturn(false); @SuppressWarnings("unchecked") Consumer> eventConsumer = mock(Consumer.class); + doReturn(false).when(service).triggerIncrementalUpdate( + testProject, "feature", "f2", "", eventConsumer); boolean result = service.ensureRagIndexUpToDate(testProject, "feature", eventConsumer); - assertThat(result).isTrue(); + assertThat(result).isFalse(); + verify(service).triggerIncrementalUpdate( + testProject, "feature", "f2", "", eventConsumer); + verify(mockVcs, never()).getBranchDiff(anyString(), anyString(), anyString(), anyString()); } @Test - void testEnsureRagIndexUpToDate_DifferentBranch_NullDiff() throws Exception { + void outdatedBranchCheckpointCannotBeSavedOutsideDurableTrigger() throws Exception { + service = spy(service); setupRagEnabled(); setupVcsBinding(); when(ragIndexTrackingService.isProjectIndexed(testProject)).thenReturn(true); @@ -1904,15 +2044,18 @@ void testEnsureRagIndexUpToDate_DifferentBranch_NullDiff() throws Exception { branchIndex.setCommitHash("f1"); when(ragBranchIndexRepository.findByProjectIdAndBranchName(100L, "feature")) .thenReturn(Optional.of(branchIndex)); - when(mockVcs.getBranchDiff("my-workspace", "my-repo", "f1", "f2")).thenReturn(null); @SuppressWarnings("unchecked") Consumer> eventConsumer = mock(Consumer.class); + doReturn(true).when(service).triggerIncrementalUpdate( + testProject, "feature", "f2", "", eventConsumer); boolean result = service.ensureRagIndexUpToDate(testProject, "feature", eventConsumer); assertThat(result).isTrue(); - // Verify that getBranchDiff was called with the commit hashes - verify(mockVcs).getBranchDiff("my-workspace", "my-repo", "f1", "f2"); + verify(service).triggerIncrementalUpdate( + testProject, "feature", "f2", "", eventConsumer); + verify(ragBranchIndexRepository, never()).save(branchIndex); + verify(mockVcs, never()).getBranchDiff(anyString(), anyString(), anyString(), anyString()); } @Test diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessor.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessor.java index 412ddf0a..6700f32e 100644 --- a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessor.java +++ b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessor.java @@ -397,8 +397,8 @@ public WebhookResult process( "message", "Posting QA-document preview links to " + taskId + "..." )); - // 8. Keep the QA guide in Jira, replacing only its large test-case - // section with the public preview URL. + // 8. Keep the compact QA guide in Jira, replacing the test-case and + // environment/setup bodies with their public-preview URLs. String commentBody = qaDocPublicPreviewService.buildTaskComment(qaDocument, previewUrl); TaskCommentVisibility visibility = toTaskCommentVisibility(qaConfig.commentVisibility()); String action; diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaAutoDocListener.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaAutoDocListener.java index 4853b522..4a7b0595 100644 --- a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaAutoDocListener.java +++ b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaAutoDocListener.java @@ -385,8 +385,8 @@ private void processQaAutoDoc(AnalysisCompletedEvent event) throws Exception { return; } - // 10. Keep the QA guide in Jira, replacing only its large test-case - // section with the public preview URL. + // 10. Keep the compact QA guide in Jira, replacing the test-case and + // environment/setup bodies with their public-preview URLs. String commentBody; try { String previewUrl = qaDocPublicPreviewService.createPreviewUrl(persistedDocument.get()); diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicy.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicy.java index 8625cc22..da27b312 100644 --- a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicy.java +++ b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicy.java @@ -2,6 +2,7 @@ import org.rostilos.codecrow.core.model.qadoc.QaDocDocument; import org.rostilos.codecrow.core.model.qadoc.QaDocState; +import org.rostilos.codecrow.core.service.qadoc.QaDocContentParser; import java.time.OffsetDateTime; import java.util.Locale; @@ -25,6 +26,7 @@ public static boolean shouldReuse( || expectedTaskId.isBlank() || document.getMarkdownContent() == null || document.getMarkdownContent().isBlank() + || !QaDocContentParser.hasCompleteShareableSections(document.getMarkdownContent()) || !Objects.equals(normalize(document.getCommitHash()), normalize(currentCommitHash)) || !Objects.equals( normalizeTaskId(document.getTaskId()), diff --git a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewService.java b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewService.java index 1943cab4..d691c625 100644 --- a/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewService.java +++ b/java-ecosystem/services/pipeline-agent/src/main/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewService.java @@ -25,9 +25,9 @@ public String createPreviewUrl(QaDocDocument document) { if (document == null || document.getId() == null) { throw new IllegalArgumentException("A persisted QA document is required for public preview."); } - if (QaDocContentParser.parseMarkedTestCases(document.getMarkdownContent()).isEmpty()) { + if (!QaDocContentParser.hasCompleteShareableSections(document.getMarkdownContent())) { throw new IllegalArgumentException( - "A marked QA test-case section is required for public preview."); + "Complete marked QA test-case and environment sections are required for public preview."); } IssuedPublicShare share = publicShareLinkService.issue( QaDocPublicShareResource.DOCUMENT, diff --git a/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessorTest.java b/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessorTest.java index caebc2e0..935b12f3 100644 --- a/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessorTest.java +++ b/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/generic/processor/command/QaDocCommandProcessorTest.java @@ -86,12 +86,19 @@ class QaDocCommandProcessorTest { ### 3. Test Scenarios + **Reject an invalid password** (HIGH) - **Expected Result:** Test steps here ### 4. Edge Cases Verify an empty password. + + + ### 6. Environment and Setup Notes + + No special setup is required. + """; @BeforeEach diff --git a/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicyTest.java b/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicyTest.java index 020f3a55..2a8e423e 100644 --- a/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicyTest.java +++ b/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocHandoffRetryPolicyTest.java @@ -39,6 +39,23 @@ void doesNotReuseBlankDocument() { document, null, "commit-a", "TASK-1")).isFalse(); } + @Test + void doesNotReuseALegacyDocumentWithoutTheCompleteSentinelContract() { + QaDocDocument document = document("commit-a", OffsetDateTime.now()); + document.setMarkdownContent(""" + + ### 3. Test Scenarios + **Legacy scenario** (HIGH) + + + ### 6. Environment and Setup Notes + Use staging. + """); + + assertThat(QaDocHandoffRetryPolicy.shouldReuse( + document, null, "commit-a", "TASK-1")).isFalse(); + } + @Test void doesNotReuseDocumentGeneratedForAnotherTask() { QaDocDocument document = document("commit-a", OffsetDateTime.now()); @@ -51,7 +68,22 @@ private static QaDocDocument document(String commit, OffsetDateTime generatedAt) QaDocDocument document = new QaDocDocument(null, 17L); document.setCommitHash(commit); document.setGeneratedAt(generatedAt); - document.setMarkdownContent("# QA document"); + document.setMarkdownContent(""" + # QA document + + + ### 3. Test Scenarios + + **Valid scenario** (HIGH) + - **Expected Result:** It works. + + + + ### 6. Environment and Setup Notes + + No special setup is required. + + """); document.setTaskId("TASK-1"); return document; } diff --git a/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewServiceTest.java b/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewServiceTest.java index 29eafb20..dc64233d 100644 --- a/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewServiceTest.java +++ b/java-ecosystem/services/pipeline-agent/src/test/java/org/rostilos/codecrow/pipelineagent/qadoc/QaDocPublicPreviewServiceTest.java @@ -39,8 +39,10 @@ void createsAnOpaquePublicLinkAndRedactsShareableBodiesFromTheTaskComment() { ### 3. Test Scenarios + **A shared scenario** (HIGH) - **Expected Result:** REDACTED TEST DETAILS + ### 4. Edge Cases PRESERVED EDGE CASE CONTENT @@ -48,9 +50,11 @@ void createsAnOpaquePublicLinkAndRedactsShareableBodiesFromTheTaskComment() { ### 5. Regression Risks PRESERVED REGRESSION RISK CONTENT + ### 6. Environment and Setup Notes + PRESERVED SETUP CONTENT - + --- *🐦 Generated by [CodeCrow](https://codecrow.app) QA Auto-Documentation* @@ -94,6 +98,6 @@ void refusesToIssueALinkForAnUnmarkedDocument() { org.assertj.core.api.Assertions.assertThatThrownBy( () -> service.createPreviewUrl(document)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("marked QA test-case section"); + .hasMessageContaining("test-case and environment sections"); } } diff --git a/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProvider.java b/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProvider.java index 07769e1b..85f84ceb 100644 --- a/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProvider.java +++ b/java-ecosystem/services/web-server/src/main/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProvider.java @@ -63,10 +63,10 @@ public Optional getAuthorizedPath(String resourceKey, Authentication aut } private Optional toPublicPreview(QaDocDocument document) { - var testCases = QaDocContentParser.parseMarkedTestCases(document.getMarkdownContent()); - if (testCases.isEmpty()) { + if (!QaDocContentParser.hasCompleteShareableSections(document.getMarkdownContent())) { return Optional.empty(); } + var testCases = QaDocContentParser.parseMarkedTestCases(document.getMarkdownContent()); QaDocContent content = QaDocContentParser.parse(document.getMarkdownContent()); String projectName = document.getProject() == null diff --git a/java-ecosystem/services/web-server/src/test/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProviderTest.java b/java-ecosystem/services/web-server/src/test/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProviderTest.java index 6af3a1f2..bf06e64c 100644 --- a/java-ecosystem/services/web-server/src/test/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProviderTest.java +++ b/java-ecosystem/services/web-server/src/test/java/org/rostilos/codecrow/webserver/publicshare/qadoc/QaDocShareProviderTest.java @@ -41,12 +41,16 @@ void returnsTheExplicitlyShareableDocumentTabs() { ### Test Scenarios + **Save the form** (HIGH) - **Expected Result:** The confirmation appears + ## 6. Setup and Environment Notes + - Enable saved cards in the QA environment. + """); CodeAnalysis analysis = mock(CodeAnalysis.class); when(analysis.getProject()).thenReturn(project); @@ -116,9 +120,16 @@ void doesNotExposeTaskSummaryFromAnotherProject() { document.setMarkdownContent(""" ### Test Scenarios + **Save the form** (HIGH) - **Expected Result:** The confirmation appears + + + ### Environment + + No special setup is required. + """); CodeAnalysis analysis = mock(CodeAnalysis.class); when(analysis.getProject()).thenReturn(analysisProject); diff --git a/python-ecosystem/inference-orchestrator/src/service/qa_documentation/qa_doc_orchestrator.py b/python-ecosystem/inference-orchestrator/src/service/qa_documentation/qa_doc_orchestrator.py index 659f5adb..83fa8a6d 100644 --- a/python-ecosystem/inference-orchestrator/src/service/qa_documentation/qa_doc_orchestrator.py +++ b/python-ecosystem/inference-orchestrator/src/service/qa_documentation/qa_doc_orchestrator.py @@ -36,7 +36,7 @@ QA_STAGE_3_AGGREGATION_PROMPT, QA_STAGE_3_DELTA_PROMPT, QA_STAGE_3_PREVIOUS_DOC_SECTION, - QA_DOC_TEST_CASES_REPAIR_PROMPT, + QA_DOC_SECTION_BOUNDARY_REPAIR_PROMPT, ) logger = logging.getLogger(__name__) @@ -44,6 +44,17 @@ # Threshold: if total diff is under this many chars, skip multi-stage and do single-pass SINGLE_PASS_THRESHOLD = 8_000 # ~2k tokens — small PRs don't need multi-stage +TEST_CASE_SENTINELS = ( + "", + "", + "", +) +ENVIRONMENT_SENTINELS = ( + "", + "", + "", +) + class QaDocOrchestrator(BaseOrchestrator): """ @@ -148,7 +159,7 @@ async def run( logger.warning("QA doc generation produced empty/short output") return {"documentation_needed": False, "documentation": None} - documentation = await self._ensure_test_cases(documentation, placeholders) + documentation = await self._ensure_shareable_sections(documentation, placeholders) documentation = self._normalize_document_title( documentation, placeholders["pr_title"], @@ -382,7 +393,7 @@ async def _process_batch(idx: int, batch: List[Dict[str, Any]]) -> Dict[str, Any try: response = await self.llm.ainvoke([ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": QA_DOC_SYSTEM_PROMPT.format(**placeholders)}, {"role": "user", "content": prompt}, ]) text = self._extract_text(response) @@ -483,7 +494,7 @@ async def _execute_stage_2( try: response = await self.llm.ainvoke([ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": QA_DOC_SYSTEM_PROMPT.format(**placeholders)}, {"role": "user", "content": prompt}, ]) content = self._extract_text(response) @@ -548,7 +559,7 @@ async def _attempt(budget: int) -> str: ) response = await self.llm.ainvoke([ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": QA_DOC_SYSTEM_PROMPT.format(**placeholders)}, {"role": "user", "content": prompt}, ]) return self._extract_text(response) @@ -609,7 +620,7 @@ async def _attempt(budget: int) -> str: ) response = await self.llm.ainvoke([ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": QA_DOC_SYSTEM_PROMPT.format(**placeholders)}, {"role": "user", "content": prompt}, ]) return self._extract_text(response) @@ -626,7 +637,7 @@ async def _attempt(budget: int) -> str: return await _attempt(BUDGET_TIGHT) response = await self.llm.ainvoke([ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": QA_DOC_SYSTEM_PROMPT.format(**placeholders)}, {"role": "user", "content": prompt}, ]) return self._extract_text(response) @@ -681,7 +692,7 @@ async def _run_single_pass( user_prompt = update_preamble + "\n\n" + user_prompt messages = [ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": QA_DOC_SYSTEM_PROMPT.format(**sp_placeholders)}, {"role": "user", "content": user_prompt}, ] response = await self.llm.ainvoke(messages) @@ -699,20 +710,14 @@ async def _run_single_pass( return content - async def _ensure_test_cases( + async def _ensure_shareable_sections( self, documentation: str, placeholders: Dict[str, str], ) -> str: - """Guarantee an independently extractable test-case section. - - Normal generation is instructed to emit stable invisible markers. If a - custom template or model response omits them, one focused repair call - generates only the missing section without narrowing the requested test - coverage. - """ - if self._contains_extractable_test_cases(documentation): - return self._normalize_test_case_markers(documentation) + """Require exact, language-independent boundaries for both shareable sections.""" + if self._has_complete_shareable_sections(documentation): + return documentation repair_placeholders = dict(placeholders) raw_diff = repair_placeholders.get("diff", "") @@ -723,123 +728,73 @@ async def _ensure_test_cases( + f"\n\n... (diff truncated — {len(raw_diff)} chars total, " f"showing first {max_repair_diff})" ) + repair_placeholders["documentation"] = documentation - logger.warning("QA doc omitted extractable test cases; running focused repair generation") - prompt = QA_DOC_TEST_CASES_REPAIR_PROMPT.format(**repair_placeholders) + logger.warning( + "QA doc violated the shareable-section sentinel contract; " + "running structural repair generation" + ) + prompt = QA_DOC_SECTION_BOUNDARY_REPAIR_PROMPT.format(**repair_placeholders) + system_prompt = QA_DOC_SYSTEM_PROMPT.format( + output_language=repair_placeholders.get("output_language", "English") + ) response = await self.llm.ainvoke([ - {"role": "system", "content": QA_DOC_SYSTEM_PROMPT}, + {"role": "system", "content": system_prompt}, {"role": "user", "content": prompt}, ]) - generated = self._extract_text(response).strip() - if not generated: - raise ValueError("Test-case generation returned no content") - - start_marker = "" - end_marker = "" - start = generated.find(start_marker) - end = generated.find(end_marker, start + len(start_marker)) if start >= 0 else -1 - if start >= 0 and end >= 0: - test_case_section = generated[start:end + len(end_marker)] - else: - test_case_section = f"{start_marker}\n{generated}\n{end_marker}" + repaired = self._extract_text(response).strip() + if not repaired: + raise ValueError("QA documentation sentinel repair returned no content") + if not self._has_complete_shareable_sections(repaired): + raise ValueError( + "QA documentation is missing complete test-case or environment sentinel sections" + ) + return repaired - if not self._contains_extractable_test_cases(test_case_section): - raise ValueError("Test-case generation returned no structured scenarios") + @classmethod + def _has_complete_shareable_sections(cls, documentation: Optional[str]) -> bool: + test_cases = cls._extract_sentinel_section(documentation, TEST_CASE_SENTINELS) + environment = cls._extract_sentinel_section(documentation, ENVIRONMENT_SENTINELS) + if test_cases is None or environment is None or test_cases[1] > environment[0]: + return False + return re.search( + r"(?mi)^\s*\*\*.+?\*\*\s*\((?:HIGH|MEDIUM|LOW)\)", + test_cases[3], + ) is not None - return self._normalize_test_case_markers( - documentation.rstrip() + "\n\n" + test_case_section.strip() - ) + @classmethod + def _contains_extractable_test_cases(cls, documentation: Optional[str]) -> bool: + test_cases = cls._extract_sentinel_section(documentation, TEST_CASE_SENTINELS) + if test_cases is None: + return False + return re.search( + r"(?mi)^\s*\*\*.+?\*\*\s*\((?:HIGH|MEDIUM|LOW)\)", + test_cases[3], + ) is not None @staticmethod - def _normalize_test_case_markers(documentation: str) -> str: - """Keep later peer sections outside the test-case disclosure boundary. + def _extract_sentinel_section( + documentation: Optional[str], + sentinels: tuple[str, str, str], + ) -> Optional[tuple[int, int, str, str]]: + """Extract one exact sentinel block without interpreting its localized heading.""" + if not documentation: + return None + start_marker, content_marker, end_marker = sentinels + if any(documentation.count(marker) != 1 for marker in sentinels): + return None - A model can occasionally place the closing marker at the end of the - document despite the prompt. Move it before the next heading at the - Test Scenarios heading level so sections such as Edge Cases, - Regression Risks, and Environment Notes remain independent. - """ - start_marker = "" - end_marker = "" start = documentation.find(start_marker) - if start < 0: - return documentation - content_start = start + len(start_marker) - end = documentation.find(end_marker, content_start) - if end < 0: - return documentation - - marked_content = documentation[content_start:end] - test_heading = re.search( - r"(?mi)^\s*(#{2,6})\s+(?:\d+\.\s*)?" - r"Test Scenarios(?:\s+by Area)?\s*$", - marked_content, - ) - if test_heading is None: - return documentation - - heading_level = len(test_heading.group(1)) - following = marked_content[test_heading.end():] - peer_heading = None - for candidate in re.finditer( - r"(?m)^\s*(#{2,6})\s+(.+?)\s*$", - following, - ): - candidate_level = len(candidate.group(1)) - candidate_title = candidate.group(2).strip() - numbered_section = re.match(r"^\d+\.\s+.+$", candidate_title) is not None - known_later_section = re.match( - r"(?i)^(?:Edge Cases(?: and Negative Testing)?|Negative Testing|" - r"Regression Risks|Environment(?: and Setup Notes)?|Setup Notes).*$", - candidate_title, - ) is not None - if ( - candidate_level < heading_level - or ( - candidate_level == heading_level - and (numbered_section or known_later_section) - ) - ): - peer_heading = test_heading.end() + candidate.start() - break - if peer_heading is None: - return documentation - - test_content = marked_content[:peer_heading].strip() - later_sections = marked_content[peer_heading:].strip() - after_marker = documentation[end + len(end_marker):].strip() - - normalized = ( - documentation[:content_start].rstrip() - + "\n" - + test_content - + "\n" - + end_marker - + "\n\n" - + later_sections - ) - if after_marker: - normalized += "\n\n" + after_marker - return normalized.strip() + content_start = documentation.find(content_marker, start + len(start_marker)) + end = documentation.find(end_marker, content_start + len(content_marker)) + if start < 0 or content_start < 0 or end < 0: + return None - @staticmethod - def _contains_extractable_test_cases(documentation: Optional[str]) -> bool: - if not documentation: - return False - start_marker = "" - end_marker = "" - start = documentation.find(start_marker) - if start < 0: - return False - content_start = start + len(start_marker) - end = documentation.find(end_marker, content_start) - if end < 0: - return False - marked_content = documentation[content_start:end] - return re.search( - r"(?mi)^\s*\*\*.+?\*\*\s*\((?:HIGH|MEDIUM|LOW)\)", - marked_content, - ) is not None + heading = documentation[start + len(start_marker):content_start].strip() + content = documentation[content_start + len(content_marker):end].strip() + if not heading.startswith("#") or "\n" in heading or not content: + return None + return start, end + len(end_marker), heading, content @staticmethod def _normalize_document_title(documentation: str, fallback_title: str) -> str: diff --git a/python-ecosystem/inference-orchestrator/src/utils/prompts/constants_qa_doc.py b/python-ecosystem/inference-orchestrator/src/utils/prompts/constants_qa_doc.py index c4b870dc..8f14b08f 100644 --- a/python-ecosystem/inference-orchestrator/src/utils/prompts/constants_qa_doc.py +++ b/python-ecosystem/inference-orchestrator/src/utils/prompts/constants_qa_doc.py @@ -50,6 +50,9 @@ 8. Focus on WHAT changed from the user's perspective, not HOW it was implemented. 9. Use markdown formatting: headings (#), bold (**), bullet lists (-), numbered lists (1.). 10. Keep it actionable — testers should be able to start testing immediately after reading. +11. Return exactly one test-case sentinel block in this order: ``, its localized section heading, ``, all test scenarios, then ``. +12. Return exactly one environment sentinel block in this order: ``, its localized section heading, ``, all environment/setup notes, then ``. This block is mandatory; when no special setup is needed, say so explicitly inside its content. +13. Never repeat test scenarios or environment/setup notes outside their sentinel blocks, including when a custom template contains similarly named sections. STRICTLY FORBIDDEN (never include any of these in the output): - File names or file paths (e.g. "src/main/...", "UserService.java", "index.tsx") @@ -129,6 +132,7 @@ Wrap the complete test-case section in these exact invisible markers: ### Test Scenarios + **Scenario Name** (HIGH) - **Preconditions:** Required setup - **Steps:** @@ -136,7 +140,14 @@ - **Expected Result:** Observable result Use exactly HIGH, MEDIUM, or LOW as each scenario's priority. -Do not place non-test-case content between those markers.""" +Do not place non-test-case content between those markers. + +Also include this mandatory environment/setup block: + +### Environment and Setup Notes + +State special setup requirements, or explicitly state that no special setup is required. +""" QA_DOC_BASE_PROMPT = """Generate structured QA documentation for the following PR changes. @@ -185,6 +196,7 @@ ### 3. Test Scenarios + For each functional area, list test scenarios using this format: **Scenario Name** (HIGH) @@ -203,8 +215,11 @@ ### 5. Regression Risks Areas of the application that might be indirectly affected. Describe by feature/screen, not by code. + ### 6. Environment and Setup Notes + Any special configuration, test data, or browser requirements needed for testing. + Generate the documentation now.""" @@ -251,6 +266,7 @@ ### Test Scenarios + **Scenario Name** (HIGH) - **Preconditions:** Required setup - **Steps:** @@ -259,7 +275,20 @@ Use exactly HIGH, MEDIUM, or LOW as each scenario's priority. -Do not place overview content between the markers. Generate the QA documentation now.""" +Do not place overview content between the markers. If the custom template contains a +test-case/scenario area, use it only as structural guidance: move that content into the +single block above and do not render a second test-case section. + +After the test-case block, append this mandatory block even when no special setup is needed: + +### Environment and Setup Notes + +State all environment/setup requirements, or explicitly state that none are required. + + +If the custom template contains environment/setup content, move it into this single +environment block and do not render a duplicate section outside the markers. +Generate the QA documentation now.""" # --------------------------------------------------------------------------- @@ -492,6 +521,7 @@ ## 3. Test Scenarios by Area + Group scenarios under the functional area heading (### Area Name). List each scenario using the bullet-list format shown above. @@ -507,11 +537,14 @@ Areas of the application that were NOT changed but might be affected: - [Feature/screen] — why it might be impacted, how to verify + ## 6. Environment and Setup Notes + Any special requirements: - Test data needed - Configuration or feature flags to enable - Browser or device requirements + Generate the FINAL document now. Remember: NO TABLES, NO FILE NAMES, NO CODE, NO SQL, NO API ENDPOINTS, NO CLI COMMANDS.""" @@ -561,16 +594,19 @@ 5. **Marking** new/updated sections with *(updated in latest push)* so testers see what's new 6. **Preserving** all scenarios from the previous guide that are still valid 7. **Stripping any technical references** that may have leaked — replace with user-facing descriptions -8. **Preserving the exact `` and `` markers** around all test scenarios. If the previous guide has no markers, add them. +8. **Returning the exact test-case sentinel sequence**: start marker, localized heading, content marker, all scenarios, end marker. +9. **Returning the exact environment sentinel sequence**: start marker, localized heading, content marker, all setup notes, end marker. Include it even when its content only states that no special setup is required. +10. **Keeping all test cases and environment/setup notes inside those blocks only**, with no duplicate sections elsewhere. The result must be a COMPLETE, standalone QA testing guide — not just the changes. Generate the updated document now. Remember: NO TABLES, NO FILE NAMES, NO CODE, NO SQL, NO API ENDPOINTS, NO CLI COMMANDS.""" -QA_DOC_TEST_CASES_REPAIR_PROMPT = """Generate ONLY the missing manual test-case section for this PR. -Write all user-facing text in **{output_language}**. Keep the existing breadth and -detail of the QA guidance; do not summarize or reduce scenarios because the change is large. +QA_DOC_SECTION_BOUNDARY_REPAIR_PROMPT = """Repair the sentinel structure of this COMPLETE QA testing guide. +Write user-facing text in **{output_language}**, but preserve the guide's content, +coverage, ordering, localized headings, and level of detail. Return the complete guide, +not an explanation and not only the repaired sections. PR #{pr_number} in {project_name}: {pr_title} Task: {task_key} — {task_summary} @@ -586,9 +622,15 @@ {diff} ``` -Return only this structure, with one or more concrete scenarios: +Existing guide: +--- +{documentation} +--- + +Required exact structure for test cases: -### Test Scenarios +### Localized test-case section heading + **Scenario Name** (HIGH) - **Preconditions:** Required setup - **Steps:** @@ -596,10 +638,21 @@ - **Expected Result:** Observable result -Use exactly HIGH, MEDIUM, or LOW as each scenario's priority. - -Never include file names, code symbols, SQL, endpoints, commands, configuration -files, code snippets, or implementation-stack details.""" +Required exact structure for environment/setup notes: + +### Localized environment/setup section heading + +All setup requirements, or an explicit statement that no special setup is required. + + +Each of the six markers must occur exactly once and in the shown order. Move all test +scenarios into the test-case block and all environment/setup notes into the environment +block. Do not duplicate either kind of content elsewhere. Preserve every unrelated +section before, between, and after these blocks. The test-case body must contain one or +more concrete scenarios with HIGH, MEDIUM, or LOW priority. + +Never include file names, code symbols, SQL, endpoints, commands, configuration files, +code snippets, or implementation-stack details.""" # --------------------------------------------------------------------------- diff --git a/python-ecosystem/inference-orchestrator/tests/test_qa_documentation.py b/python-ecosystem/inference-orchestrator/tests/test_qa_documentation.py index 19a454e8..88c50506 100644 --- a/python-ecosystem/inference-orchestrator/tests/test_qa_documentation.py +++ b/python-ecosystem/inference-orchestrator/tests/test_qa_documentation.py @@ -219,14 +219,21 @@ def test_compact_format(self): class TestIndependentTestCases: - def test_custom_template_cannot_remove_test_cases(self): + def test_custom_template_requires_both_exact_sentinel_blocks(self): assert "custom template MUST NOT remove test cases" in QA_DOC_CUSTOM_PROMPT assert "" in QA_DOC_CUSTOM_PROMPT + assert "" in QA_DOC_CUSTOM_PROMPT assert "" in QA_DOC_CUSTOM_PROMPT + assert "" in QA_DOC_CUSTOM_PROMPT + assert "" in QA_DOC_CUSTOM_PROMPT + assert "" in QA_DOC_CUSTOM_PROMPT + assert "do not render a duplicate" in QA_DOC_CUSTOM_PROMPT def test_detects_only_marked_structured_scenarios(self): valid = """ + ### Будь-який локалізований заголовок + **Checkout succeeds** (HIGH) - **Expected Result:** Confirmation appears @@ -237,6 +244,8 @@ def test_detects_only_marked_structured_scenarios(self): ) assert not QaDocOrchestrator._contains_extractable_test_cases(""" + ### Test Scenarios + **Scenario outside the disclosure boundary** (HIGH) """) @@ -244,49 +253,99 @@ def test_detects_only_marked_structured_scenarios(self): **Scenario between reversed markers** (HIGH) + ### Test Scenarios + """) - def test_moves_an_overbroad_end_marker_before_later_numbered_sections(self): - documentation = """ - ### 1. Change Summary - Checkout changed. - + def test_accepts_complete_blocks_with_arbitrary_localized_headings(self): + valid = """ - ### 3. Test Scenarios - ### Checkout + ## Абсолютно довільний локалізований заголовок + + ### Оформлення + **Успішне оформлення** (HIGH) + - **Expected Result:** Замовлення створено + + + ## 4. Граничні випадки та негативне тестування + - Перевірити порожній кошик. + + + ## Ще один довільний локалізований заголовок + + - Використати тестове середовище. + + """ + + assert QaDocOrchestrator._has_complete_shareable_sections(valid) + + def test_does_not_infer_sections_from_headings(self): + unmarked = """ + ## 3. Test Scenarios **Checkout succeeds** (HIGH) - **Expected Result:** Confirmation appears + ## 6. Налаштування та вимоги до середовища + - Використати тестове середовище. + """ - ### 4. Edge Cases and Negative Testing - - Try an expired card. - - ### 5. Regression Risks - - Existing card payments. + assert not QaDocOrchestrator._has_complete_shareable_sections(unmarked) + assert not QaDocOrchestrator._contains_extractable_test_cases(unmarked) - ### 6. Environment and Setup Notes - - Use the QA environment. + def test_rejects_duplicate_or_misordered_sentinels(self): + invalid = """ + + ### Setup + + No special setup. + + + + ### Tests + + **Checkout succeeds** (HIGH) """ - normalized = QaDocOrchestrator._normalize_test_case_markers(documentation) + assert not QaDocOrchestrator._has_complete_shareable_sections(invalid) - assert normalized.index("") < normalized.index( - "### 4. Edge Cases and Negative Testing" - ) - assert normalized.index("") > normalized.index( - "**Checkout succeeds** (HIGH)" - ) - assert "### 5. Regression Risks" in normalized - assert "### 6. Environment and Setup Notes" in normalized + def test_rejects_body_text_in_the_heading_slot(self): + invalid = """ + + ### Tests + This must not leak into Jira before the link. + + **Checkout succeeds** (HIGH) + + + ### Setup + + No special setup. + + """ + + assert not QaDocOrchestrator._has_complete_shareable_sections(invalid) @pytest.mark.asyncio(loop_scope="function") - async def test_repairs_a_custom_document_that_omits_test_cases(self): + async def test_repairs_the_complete_document_when_sentinels_are_missing(self): response = MagicMock(content=""" + # Custom QA summary + The checkout flow changed. + - ### Test Scenarios + ### Тестові сценарії + **Checkout succeeds** (HIGH) - **Expected Result:** Confirmation appears + + ## 4. Edge Cases + Try an expired card. + + + ### Середовище + + No special setup is required. + """) llm = MagicMock() llm.ainvoke = AsyncMock(return_value=response) @@ -305,16 +364,30 @@ async def test_repairs_a_custom_document_that_omits_test_cases(self): "diff": "+ changed behavior", } - repaired = await orchestrator._ensure_test_cases("# Custom QA summary", placeholders) + repaired = await orchestrator._ensure_shareable_sections( + "# Custom QA summary\n\nThe checkout flow changed.", + placeholders, + ) assert repaired.startswith("# Custom QA summary") - assert "" in repaired + assert "" in repaired + assert "" in repaired assert "**Checkout succeeds** (HIGH)" in repaired + assert "## 4. Edge Cases" in repaired + repair_prompt = llm.ainvoke.await_args.args[0][1]["content"] + assert "# Custom QA summary" in repair_prompt + assert "Each of the six markers must occur exactly once" in repair_prompt @pytest.mark.asyncio(loop_scope="function") - async def test_rejects_a_repair_without_a_structured_scenario(self): + async def test_rejects_a_repair_without_both_complete_blocks(self): llm = MagicMock() - llm.ainvoke = AsyncMock(return_value=MagicMock(content="General testing notes")) + llm.ainvoke = AsyncMock(return_value=MagicMock(content=""" + + ### Tests + + **Checkout succeeds** (HIGH) + + """)) orchestrator = QaDocOrchestrator(llm=llm) placeholders = { "output_language": "English", @@ -330,8 +403,8 @@ async def test_rejects_a_repair_without_a_structured_scenario(self): "diff": "+ changed behavior", } - with pytest.raises(ValueError, match="no structured scenarios"): - await orchestrator._ensure_test_cases("# Custom QA summary", placeholders) + with pytest.raises(ValueError, match="missing complete test-case or environment"): + await orchestrator._ensure_shareable_sections("# Custom QA summary", placeholders) # ── QaDocOrchestrator._build_placeholders ────────────────────────