fix(reliability): stabilize analysis queues and RAG lifecycle - #249
Conversation
rostilos
commented
Aug 12, 2026
rostilos
commented
- renew and fence PR, branch, exact-generation, and legacy RAG leases
- recover abandoned jobs without corrupting newer owners or checkpoints
- fix detached RAG generation proxies by using scalar projections
- make exact generation admission, publication, and status recovery durable
- preserve legacy Qdrant collections and repair payload indexes in place
- harden PR overlay cleanup, alias reconciliation, and transient cleanup
- handle excluded incremental files without failing the indexing job
- treat lock contention as a skipped update instead of a failure
- drain admitted queue and streaming work during graceful shutdown
- serialize terminal events and add bounded Redis event retention
- degrade optional RAG context without failing the core review
- remove unused QA documentation RAG overlays and credential forwarding
- suppress duplicate diagnostics while retaining owner-level failures
- add recovery configuration samples, migration, and regression coverage
- renew and fence PR, branch, exact-generation, and legacy RAG leases - recover abandoned jobs without corrupting newer owners or checkpoints - fix detached RAG generation proxies by using scalar projections - make exact generation admission, publication, and status recovery durable - preserve legacy Qdrant collections and repair payload indexes in place - harden PR overlay cleanup, alias reconciliation, and transient cleanup - handle excluded incremental files without failing the indexing job - treat lock contention as a skipped update instead of a failure - drain admitted queue and streaming work during graceful shutdown - serialize terminal events and add bounded Redis event retention - degrade optional RAG context without failing the core review - remove unused QA documentation RAG overlays and credential forwarding - suppress duplicate diagnostics while retaining owner-level failures - add recovery configuration samples, migration, and regression coverage
|
| Status | PASS WITH WARNINGS |
| Risk Level | MEDIUM |
| Review Coverage | 137 files analyzed in depth |
| Confidence | HIGH |
Executive Summary
This PR makes broad reliability changes across Java analysis/RAG services and Python queue processing, including queue ownership, heartbeats, recovery, generation lifecycle, and shutdown behavior. The overall direction is sound, but the breadth of the changes introduces moderate integration risk around cross-service state transitions, error handling, and lifecycle consistency. No critical or high-severity blockers were identified; several medium-severity concerns should be addressed or tracked before relying on the new reliability behavior in production.
Recommendation
Decision: PASS WITH WARNINGS
The PR may proceed with the documented warnings, provided the identified lifecycle and error-handling concerns receive follow-up, particularly around ensuring published work and associated jobs reach consistent terminal states. Additional integration testing across queue recovery, exact-generation bootstrap, ownership transfer, and shutdown scenarios is recommended.
Issues Overview
| Severity | Count | |
|---|---|---|
| 🟡 Medium | 5 | Issues that should be addressed |
Analysis completed on 2026-08-12 18:49:59 | View Full Report | Pull Request
📋 Detailed Issues (5)
🟡 Medium Severity Issues
Id on Platform: 4049
Category: 🐛 Bug Risk
File: .../service/RagOperationsServiceImpl.java:306
Initial exact generation is gated out
In exact-generation mode, the method proceeds only when normalIncrementalReady or exactRecoveryCandidate is true. exactRecoveryCandidate requires an existing branch-index registry row, so a project with no exact-generation row cannot reach the later admit(...) path that creates that first generation whenever the legacy incremental readiness check is false. This leaves a newly configured exact-generation project unable to bootstrap its initial index.
💡 Suggested Fix
Allow exact-generation requests to proceed when RAG is enabled even if no branch-index row exists, while retaining the existing readiness gate for the legacy shared-collection path. The subsequent admission logic can then create the initial registry entry.
Id on Platform: 4050
Category: 🛡️ Error Handling
File: .../branch/BranchIndexMaintenanceService.java:243
Published build can leave job running
publicationCompleted is set immediately after generationBuildService.execute(...) returns, but before reconcilePublishedGeneration(...) and jobService.completeJob(...) run. If either projection reconciliation or job completion throws, the publicationCompleted branch rethrows the failure and deliberately skips failJob(...) and completeJob(...). The durable generation has already been published, while the admitted job remains in its running state with no terminal transition in this method, leaving job status and the published operation inconsistent until an external recovery path happens to repair it.
💡 Suggested Fix
Ensure the admitted job receives a durable terminal outcome when post-publication projection finalization fails. For example, explicitly complete or mark the job as awaiting projection recovery, or invoke a dedicated recovery-safe terminalization operation before rethrowing. Keep the generation marked successful while recording the projection failure separately.
Id on Platform: 4051
Category: 🛡️ Error Handling
File: .../branch/BranchIndexMaintenanceService.java:269
Post-publication failure bypasses job terminalization
The publicationCompleted flag covers all exceptions after execute, including failures from reconcilePublishedGeneration and completeJob. In that branch the method logs and throws without calling jobService.completeJob or jobService.failJob. This creates a concrete stuck-job path whenever database projection or job persistence fails after successful publication.
💡 Suggested Fix
Separate generation publication from subsequent projection and job-finalization steps. On post-publication errors, persist an explicit recoverable terminal state for the job or invoke a durable reconciliation routine that guarantees the job cannot remain RUNNING indefinitely.
Id on Platform: 4052
Category: 🐛 Bug Risk
File: .../api/models.py:51
Ownership transfer flag is ignored
IndexRequest now accepts transfer_repo_ownership, but the current RAGQueueConsumer indexing call forwards preserve_other_branches and the other request fields without forwarding this new flag. As a result, callers can request repository ownership transfer while the indexing operation never receives that request, so the option has no effect and ownership cleanup/transfer behavior can be incorrect.
💡 Suggested Fix
Forward request_dto.transfer_repo_ownership from RAGQueueConsumer into RAGIndexManager.index_repository, and ensure the manager's method signature and implementation consume the value.
Id on Platform: 4053
Category: 🛡️ Error Handling
File: .../branch/BranchIndexMaintenanceService.java:243
Published exact-generation work can remain non-terminal across maintenance and recovery boundaries
Published exact-generation work can remain non-terminal across maintenance and recovery boundaries
publicationCompleted is set before projection reconciliation and job terminalization. If either subsequent operation fails, the maintenance service rethrows through the post-publication path instead of completing or failing the admitted job. This leaves the published generation and job lifecycle in different states, while the separate recovery paths must infer whether the job is recoverable.
Evidence: BranchIndexMaintenanceService marks publication complete immediately after execution, and the post-publication exception path skips normal JobService terminalization. The PR separately introduces exact-operation recovery and legacy-job recovery, but those recovery mechanisms classify work by operation/job state and do not make the failed post-publication transition atomic. A database or job-service failure after successful publication can therefore leave a running job associated with an already-published generation.
Business impact: Exact RAG builds can publish successfully while their jobs remain stuck in RUNNING, producing incorrect operational status, repeated recovery attempts, and inconsistent generation/job state.
Also affects: java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/service/JobService.java, java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/branch/RagIndexOperationRecoveryService.java
💡 Suggested Fix
Make post-publication completion durable and retryable: either persist a terminal publication outcome before returning, or catch reconciliation/job-terminalization failures and enqueue an explicit recovery record. Ensure the exact-operation recovery path can deterministically complete or reconcile the associated job without guessing from a partially updated projection.
Files Affected
- .../branch/BranchIndexMaintenanceService.java: 3 issues
- .../service/RagOperationsServiceImpl.java: 1 issue
- .../api/models.py: 1 issue
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codecrow-local[bot]](https://avatars.githubusercontent.com/u/85498741?s=60&v=4){kind=small}
| boolean exactGenerationMode = usesExactGenerations(project); | ||
| boolean normalIncrementalReady = | ||
| incrementalRagUpdateService.shouldPerformIncrementalUpdate(project); | ||
| boolean exactRecoveryCandidate = exactGenerationMode |
There was a problem hiding this comment.
🟡 MEDIUM | Bug Risk
Initial exact generation is gated out
In exact-generation mode, the method proceeds only when normalIncrementalReady or exactRecoveryCandidate is true. exactRecoveryCandidate requires an existing branch-index registry row, so a project with no exact-generation row cannot reach the later admit(...) path that creates that first generation whenever the legacy incremental readiness check is false. This leaves a newly configured exact-generation project unable to bootstrap its initial index.
💡 Suggested fix
Allow exact-generation requests to proceed when RAG is enabled even if no branch-index row exists, while retaining the existing readiness gate for the legacy shared-collection path. The subsequent admission logic can then create the initial registry entry.
| } | ||
| emitEvent(events, forwarded); | ||
| }); | ||
| publicationCompleted = true; |
There was a problem hiding this comment.
🟡 MEDIUM | Error Handling
Published build can leave job running
publicationCompleted is set immediately after generationBuildService.execute(...) returns, but before reconcilePublishedGeneration(...) and jobService.completeJob(...) run. If either projection reconciliation or job completion throws, the publicationCompleted branch rethrows the failure and deliberately skips failJob(...) and completeJob(...). The durable generation has already been published, while the admitted job remains in its running state with no terminal transition in this method, leaving job status and the published operation inconsistent until an external recovery path happens to repair it.
💡 Suggested fix
Ensure the admitted job receives a durable terminal outcome when post-publication projection finalization fails. For example, explicitly complete or mark the job as awaiting projection recovery, or invoke a dedicated recovery-safe terminalization operation before rethrowing. Keep the generation marked successful while recording the projection failure separately.
| + "project={}, branch={}, job={}", | ||
| project.getId(), branch, job != null ? job.getId() : null, | ||
| failure); | ||
| throw failure instanceof RuntimeException runtime ? runtime |
There was a problem hiding this comment.
🟡 MEDIUM | Error Handling
Post-publication failure bypasses job terminalization
The publicationCompleted flag covers all exceptions after execute, including failures from reconcilePublishedGeneration and completeJob. In that branch the method logs and throws without calling jobService.completeJob or jobService.failJob. This creates a concrete stuck-job path whenever database projection or job persistence fails after successful publication.
💡 Suggested fix
Separate generation publication from subsequent projection and job-finalization steps. On post-publication errors, persist an explicit recoverable terminal state for the job or invoke a durable reconciliation routine that guarantees the job cannot remain RUNNING indefinitely.
| publish_legacy_project_alias: bool = False | ||
| preserve_other_branches: bool = False | ||
| cleanup_repo_path: bool = False | ||
| transfer_repo_ownership: bool = False |
There was a problem hiding this comment.
🟡 MEDIUM | Bug Risk
Ownership transfer flag is ignored
IndexRequest now accepts transfer_repo_ownership, but the current RAGQueueConsumer indexing call forwards preserve_other_branches and the other request fields without forwarding this new flag. As a result, callers can request repository ownership transfer while the indexing operation never receives that request, so the option has no effect and ownership cleanup/transfer behavior can be incorrect.
💡 Suggested fix
Forward request_dto.transfer_repo_ownership from RAGQueueConsumer into RAGIndexManager.index_repository, and ensure the manager's method signature and implementation consume the value.
| } | ||
| emitEvent(events, forwarded); | ||
| }); | ||
| publicationCompleted = true; |
There was a problem hiding this comment.
🟡 MEDIUM | Error Handling
Published exact-generation work can remain non-terminal across maintenance and recovery boundaries
Published exact-generation work can remain non-terminal across maintenance and recovery boundaries
publicationCompleted is set before projection reconciliation and job terminalization. If either subsequent operation fails, the maintenance service rethrows through the post-publication path instead of completing or failing the admitted job. This leaves the published generation and job lifecycle in different states, while the separate recovery paths must infer whether the job is recoverable.
Evidence: BranchIndexMaintenanceService marks publication complete immediately after execution, and the post-publication exception path skips normal JobService terminalization. The PR separately introduces exact-operation recovery and legacy-job recovery, but those recovery mechanisms classify work by operation/job state and do not make the failed post-publication transition atomic. A database or job-service failure after successful publication can therefore leave a running job associated with an already-published generation.
Business impact: Exact RAG builds can publish successfully while their jobs remain stuck in RUNNING, producing incorrect operational status, repeated recovery attempts, and inconsistent generation/job state.
Also affects: java-ecosystem/libs/core/src/main/java/org/rostilos/codecrow/core/service/JobService.java, java-ecosystem/libs/rag-engine/src/main/java/org/rostilos/codecrow/ragengine/branch/RagIndexOperationRecoveryService.java
💡 Suggested fix
Make post-publication completion durable and retryable: either persist a terminal publication outcome before returning, or catch reconciliation/job-terminalization failures and enqueue an explicit recovery record. Ensure the exact-operation recovery path can deterministically complete or reconcile the associated job without guessing from a partially updated projection.
1 participant
