You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit ecf954f
Browse filesBrowse the repository at this point in the historyBrowse files
authored
fix(brain): unwedge suggestion approval and refresh knowledge counts 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>
0 commit comments