fix: Review queue approvals (CODE_DERIVED + stale list + bulk errors) - #74
Merged
Merged
Conversation
Customer saw "198 awaiting" with only 2 rows and "APPROVED 0 OF 2" because: 1) schema_documentation CHECK lacked CODE_DERIVED on self-host (Hibernate never rewrites CHECKs) so SCHEMA_DOC approve failed inside bulk-decide 2) Review list cache stayed stale after scans while the badge refetched totals 3) bulk success toast treated succeeded=0 as success with no failure details - Startup initializer keeps source CHECK aligned with DocumentationSource - Bulk decide returns failures[] with per-id errors; UI shows them as errors - Invalidate suggestions when a scan reaches COMPLETED/FAILED - Throw on malformed SCHEMA_DOC targets instead of silent skip - Seed + E2E scripts for review queue edge cases Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
… chat - Show code-scan suggestions as clickable bubbles with rule excerpt preview - Fold scan proposals into one intent when they overlap existing knowledge or schema notes - Approve merges into existing entries/docs instead of creating duplicates - Clarify review is optional; Agent chat shows a non-blocking pending hint Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
…blocking chat" This reverts commit b74b90e.
venkateshsakamuri-lab
added a commit
that referenced
this pull request
Aug 22, 2026
…live (#77) Follow-up to #74. That PR made the failure *visible* (bulk decide now returns `failures[]`, and the UI renders a partial/zero success as an error) but the approval itself still threw — its diagnosis, a `schema_documentation_source_check` missing `CODE_DERIVED`, did not apply to the reporting install, where the CHECK was already correct and the log said something else entirely. ## What was actually failing ``` WARN CodeScanService : bulk decide skipped e2011614-…: Query did not return a unique result: 2 results were returned WARN CodeScanService : bulk decide skipped f3689e0e-…: Query did not return a unique result: 2 results were returned ``` `schema_documentation` carried no unique constraint on `(connection_id, object_type, object_name, parent_object, source)` and `CodeSuggestionApplier.approve` took no row lock, so **one bulk approve submitted twice concurrently wrote 219 duplicate pairs** (pairs ~3s apart; in every group the older row is an orphan and the newer one holds the `applied_doc_id`). Every later approve landing on such a key threw out of the `Optional`-returning upsert finder, and bulk-decide swallowed it per item. The duplicate never self-heals, so **all 198 pending SCHEMA_DOC suggestions were permanently unapprovable**. ## Fix **1. Duplicate-tolerant upsert.** The three finders now return `List`, so the compiler forces every caller to handle N matches. `SchemaDocumentationDeduplicator` keeps the newest row, repoints any `applied_doc_id` off the rows it deletes (a loose reference, not an FK — a dangling value fails silently), and drops their RAG embeddings. Applied at all four call sites, including `SchemaDriftListener`, which would have thrown identically the first time one of 17 duplicated tables was dropped. **2. Data repair + constraint.** `V116__dedupe_schema_documentation.sql`, applied by `SchemaDocumentationDedupeInitializer` — there is no Flyway runtime here, so a SQL file alone would never run. Idempotent: it returns before touching a row once the index exists. `coalesce(parent_object,'')` in the key because Postgres treats NULLs as distinct. **3. The root cause.** `approve`/`reject` load the suggestion `FOR UPDATE`, so the concurrent double-submit blocks and the second caller sees `APPROVED`. ## Also fixed (reported after the first fix landed) - **Counts stale until reload.** An approval also writes `schema_documentation`, served by `brain/notes`, which backs the Write-notes tab and its coverage counts. The decide hooks invalidated only `codeScan` + `companyKnowledge`. `invalidateAfterDecision` now covers `brain` and `schemaContext` too. - **Newest notes sorted last.** `@PreUpdate` never fires on insert, so a new note has a null `updatedAt`; sorting on it alone with nulls last sent every brand-new note to the *bottom*. Now `COALESCE(updatedAt, createdAt)`, matching `CompanyKnowledgeEntryRepository`. - **Fresh approvals buried in the Approved view.** `listSuggestions` sorted every status by confidence. PENDING stays confidence-first (it is a work queue); decided statuses now sort by `decidedAt DESC NULLS LAST`. ## Two bugs in #74's own test tooling - Both scripts hardcoded `sudo -u postgres psql`, which does not exist on the Compose deployment `install.sh` produces — the verify command in #74's description failed before testing anything. Now resolved through `scripts/self-host/vaultdb.py`. - **`e2e-review-approvals.py` step 10 rewrote every real `CODE_DERIVED` row to `source='USER'` and never restored it.** Running it against a live install silently relabelled 339 approved docs, and post-V116 it would collide with the unique index. It now parks rows in a scratch table and restores them with a verified count, and its cleanup deletes the planted row only while nothing references it. ## Verification Run against the live self-host stack, not just unit tests. - Initializer on the affected install: `removed 219 duplicate rows, 219 orphaned embeddings`; 7068 → 6849 doc rows, 8190 → 7971 RAG rows, **0 duplicate groups, 0 dangling `applied_doc_id`**. Skipped cleanly on restart. - The two originally stuck suggestions: `{"requested": 2, "succeeded": 2, "failed": 0}`. - Ordering, approving the three *lowest*-confidence items so the two sorts disagree — all three land above older `conf=0.97` rows. - `brain/notes` 4607 → 4608 on a first-time doc (the count that never refreshed), and the just-approved `crm.customers` now sorts first. - 21 backend unit tests green (4 new on collapse, 2 new on note ordering). 184 related tests run; the 6 failures in `TrainingServiceBusinessTermTest` / `BrainInitStageExecutorTest` reproduce identically on pristine `main` and are unrelated. - **E2E suite: 30/30 pass**, including duplicate collapse, `applied_doc_id` repointing, the unique index rejecting a second row, and two genuinely concurrent approves writing exactly one row. ```bash python3 scripts/self-host/seed-review-suggestions.py <connectionId> --count 20 python3 scripts/self-host/e2e-review-approvals.py <connectionId> # ✓ All review-approval edge cases passed ``` Note: the suite consumes its own fixtures, so re-run the seed before each run. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018mG2xj9gWJ8WzfDP2fDePP Co-authored-by: Krishna Sasank Talasila <606482+geekypunk@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Venkat SF <venkatesh.sakamuri@stayflexi.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (customer screenshot)
Step 3 showed 198 awaiting sign-off, the table showed 2 pending SCHEMA rows, and bulk approve displayed APPROVED 0 OF 2.
Root causes (reproduced)
schema_documentation_source_checkoften still lacksCODE_DERIVED(Hibernateddl-auto=updatenever rewrites CHECKs). SCHEMA_DOC approve insertssource=CODE_DERIVED→ constraint violation → bulk-decide swallows the error → UI showsApproved 0 of Nas a success banner.totalElementsprobe; the Review list useduseAllCodeScanSuggestionswithstaleTime: 30sand only invalidated on scan start, not on COMPLETED.{requested, succeeded}; UI treatedsucceeded: 0as success.Fix
SchemaDocumentationSourceCompatibilityInitializer) keeps the source CHECK aligned withDocumentationSource(same pattern as Brain init stages).failed+failures[{id,error}]; Review UI treats partial/zero success as an error banner with details.staleTime.CodeSuggestionApplierTest+ seed/E2E scripts:scripts/self-host/seed-review-suggestions.pyscripts/self-host/e2e-review-approvals.py(15 edge cases, all green)Verify
python3 scripts/self-host/seed-review-suggestions.py --count 20 python3 scripts/self-host/e2e-review-approvals.py # expect: ✓ All review-approval edge cases passedPlease merge the AGENTS.md updates so future agents remember the Review seed/E2E path and the CODE_DERIVED CHECK footgun.