fix(brain): unwedge suggestion approval and refresh knowledge counts live

.gitignore

@@ -104,3 +104,7 @@ optd-sidecar/target/
 *.iml
 .local-admin-credentials
 .local-mcp-token
+
+# Python bytecode from scripts/
+__pycache__/
+*.pyc

CLAUDE.md

@@ -296,6 +296,16 @@ broken. Assert the *outcome*, never the attempt:
 - **Mocks hide SDK breaks.** `tests/tools/test_mcp_structured_content.py` uses a
   `_FakeCallToolResult` with a hardcoded `.isError`, so it kept passing precisely when
   the real SDK stopped matching. Pin the dependency; a fake cannot catch this.
+- **A self-host verification script must reach the DB the way the install does.**
+  `seed-review-suggestions.py` / `e2e-review-approvals.py` hardcoded
+  `sudo -u postgres psql`, which only exists on a bare-metal install — on the Compose
+  deployment `install.sh` actually produces, the documented verify command died before
+  testing anything. Both now resolve the path through `scripts/self-host/vaultdb.py`.
+- **A test that mutates shared state must restore it, and only what it created.** The
+  same e2e suite parked every real `CODE_DERIVED` row by rewriting `source` to `USER`
+  and never restored it, so a run against a live install silently relabelled the user's
+  approved docs. It now copies rows to a scratch table and restores them, and its
+  cleanup deletes the planted row only while nothing references it.
 - **Never claim a check you did not run.** `install.sh` reported "up to date" when it
   could not reach npm; it now says it could not check.
 - **`set -e` + `read` at EOF aborts silently.** Prompts in `install.sh` use
@@ -384,6 +394,53 @@ it against a real database — not a theoretical hardening pass.
   `POST /users/admin/reset` on every install that had run `setup-agent.sh`, since that
   mints an admin MCP token on each run.
 
+- **An `Optional`-returning derived finder is an assertion that the key is unique.**
+  Spring Data throws `IncorrectResultSizeDataAccessException` ("Query did not return a
+  unique result: N results were returned") the moment it is not, and the row that broke
+  it never repairs itself, so the failure is permanent rather than transient.
+  `schema_documentation` had 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. Every later approve touching one of those keys
+  threw, `CodeScanService.bulkDecide` swallowed it per item, and the Review queue
+  reported "Approved 0 of 2" — with all 198 pending SCHEMA_DOC suggestions wedged.
+  Three-part fix, and all three are load-bearing:
+  1. `V116__dedupe_schema_documentation.sql` + `SchemaDocumentationDedupeInitializer`
+     (no Flyway here, so the initializer is what actually applies it) collapse the
+     duplicates and add `ux_schema_doc_target`, keyed on
+     `coalesce(parent_object,'')` because Postgres treats NULLs as distinct.
+  2. Those finders now return `List`, and `SchemaDocumentationDeduplicator.collapse`
+     keeps the newest row, repoints any `applied_doc_id` off the rows it deletes, and
+     drops their RAG embeddings. Do not restore an `Optional` variant — legacy installs
+     still carry duplicates until the initializer runs.
+  3. `approve`/`reject` load the suggestion via `findByIdForUpdate` (`PESSIMISTIC_WRITE`)
+     so the concurrent double-submit that created the duplicates blocks instead of racing.
+- **`applied_doc_id` is a loose reference, not an FK.** Deleting a `schema_documentation`
+  row it points at raises nothing and dangles silently — repoint before deleting.
+- **Approve *updates* the row an earlier scan wrote**, so a "freshly approved" doc row
+  carries a historical `created_at`. A test that plants an "old" duplicate with a
+  hardcoded past date can easily plant the *newer* of the two and assert nothing; anchor
+  fixture timestamps to the real row's `created_at`.
+- **A write's blast radius decides what to invalidate, not the endpoint you called.**
+  Approving a code-scan suggestion writes `code_knowledge_suggestion` *and*
+  `schema_documentation` (served by `brain/notes`, which backs the Write-notes tab
+  and its coverage counts) *and* `rag_documents` *and*, for KNOWLEDGE_ENTRY, a
+  company knowledge entry. The decide hooks invalidated only `codeScan` +
+  `companyKnowledge`, so every schema-doc-derived count stayed stale until the user
+  reloaded the page. `invalidateAfterDecision` in `useCodeScan.js` is the single
+  place that lists them; add to it when an approval starts writing something new.
+- **`@PreUpdate` does not fire on insert, so `updatedAt` is null on a brand-new row.**
+  Sorting "newest first" on `updatedAt` alone with nulls last therefore sends every
+  freshly created row to the *bottom* — which is why a just-approved note did not
+  appear at the top of the Write-notes list. Sort on
+  `COALESCE(updatedAt, createdAt)` (`BrainNoteService.touchedAt`,
+  `CompanyKnowledgeEntryRepository.findByConnectionIdOrderByRecency`).
+- **Suggestion list order depends on the status being viewed.** PENDING is a work
+  queue → `confidence DESC`. APPROVED/REJECTED are history → `decidedAt DESC NULLS
+  LAST` so the decision you just made is at the top; confidence-sorting a decided
+  list scattered fresh approvals among hundreds of older ones
+  (`CodeScanService.sortFor`).
+
 ### Endpoint Authorization Rules
 
 - **Authentication is not authorization.** `SecurityConfig` only asserts

backend/src/main/java/com/dbaagent/config/SchemaDocumentationDedupeInitializer.java

@@ -0,0 +1,116 @@
+package com.dbaagent.config;
+
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.DependsOn;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.transaction.support.TransactionTemplate;
+import org.springframework.transaction.PlatformTransactionManager;
+
+import javax.sql.DataSource;
+
+/**
+ * Applies {@code V116__dedupe_schema_documentation.sql} at startup: collapses
+ * duplicate {@code schema_documentation} rows and adds the unique index on the
+ * logical key.
+ *
+ * <p>This repo has no Flyway runtime — {@code db/migration} is a hand-maintained
+ * changelog and Hibernate {@code ddl-auto=update} never adds an index the entity
+ * does not declare. Without this, self-host installs carrying duplicates from a
+ * double-submitted bulk approve stay wedged: every SCHEMA_DOC approve throws
+ * {@code Query did not return a unique result}. Mirrors
+ * {@link SchemaDocumentationSourceCompatibilityInitializer}.
+ *
+ * <p>Idempotent and cheap on a clean install: the index exists, so it returns
+ * before touching a row.
+ */
+@Configuration
+@Slf4j
+public class SchemaDocumentationDedupeInitializer {
+
+    private static final String TABLE = "schema_documentation";
+    private static final String INDEX = "ux_schema_doc_target";
+
+    @Bean("schemaDocumentationDedupeBootstrap")
+    @DependsOn("entityManagerFactory")
+    public Object schemaDocumentationDedupeBootstrap(DataSource dataSource,
+                                                     PlatformTransactionManager txManager) {
+        JdbcTemplate jdbc = new JdbcTemplate(dataSource);
+        if (!tableExists(jdbc, TABLE)) {
+            return new Object();
+        }
+        if (indexExists(jdbc, INDEX)) {
+            return new Object();
+        }
+
+        // One transaction: a half-applied dedupe (rows deleted, index missing)
+        // would silently re-accumulate duplicates until the next boot.
+        new TransactionTemplate(txManager).executeWithoutResult(status -> {
+            int repointed = jdbc.update("""
+                UPDATE code_knowledge_suggestion s
+                SET applied_doc_id = l.keep_id
+                FROM (%s) l
+                WHERE s.applied_doc_id = l.id
+                """.formatted(LOSERS));
+
+            int embeddings = jdbc.update(
+                "DELETE FROM rag_documents WHERE id IN (SELECT id FROM (%s) l)".formatted(LOSERS));
+
+            int removed = jdbc.update(
+                "DELETE FROM schema_documentation WHERE id IN (SELECT id FROM (%s) l)".formatted(LOSERS));
+
+            jdbc.execute("""
+                CREATE UNIQUE INDEX IF NOT EXISTS %s
+                ON %s (connection_id, object_type, object_name, coalesce(parent_object, ''), source)
+                """.formatted(INDEX, TABLE));
+
+            if (removed > 0) {
+                log.warn("Deduped {}: removed {} duplicate rows, {} orphaned embeddings, "
+                        + "repointed {} applied_doc_id references",
+                    TABLE, removed, embeddings, repointed);
+            }
+            log.info("Ensured unique index {} on {}", INDEX, TABLE);
+        });
+        return new Object();
+    }
+
+    /**
+     * Every row but the newest within each logical key. Newest wins because it is
+     * the row existing {@code applied_doc_id} references point at; {@code id}
+     * breaks ties for rows written in the same clock tick. {@code coalesce} on
+     * {@code parent_object} because Postgres treats NULLs as distinct, so TABLE
+     * rows would otherwise never group together.
+     */
+    private static final String LOSERS = """
+        SELECT id, keep_id FROM (
+            SELECT id,
+                   first_value(id) OVER w AS keep_id,
+                   row_number()    OVER w AS rn
+            FROM schema_documentation
+            WINDOW w AS (
+                PARTITION BY connection_id, object_type, object_name,
+                             coalesce(parent_object, ''), source
+                ORDER BY created_at DESC NULLS LAST, id DESC
+            )
+        ) ranked WHERE rn > 1
+        """;
+
+    private boolean tableExists(JdbcTemplate jdbc, String tableName) {
+        Integer count = jdbc.queryForObject("""
+                SELECT COUNT(*)
+                FROM information_schema.tables
+                WHERE table_schema = 'public' AND table_name = ?
+                """, Integer.class, tableName);
+        return count != null && count > 0;
+    }
+
+    private boolean indexExists(JdbcTemplate jdbc, String indexName) {
+        Integer count = jdbc.queryForObject("""
+                SELECT COUNT(*)
+                FROM pg_indexes
+                WHERE schemaname = 'public' AND indexname = ?
+                """, Integer.class, indexName);
+        return count != null && count > 0;
+    }
+}

backend/src/main/java/com/dbaagent/repository/CodeKnowledgeSuggestionRepository.java

@@ -3,14 +3,44 @@
 import com.dbaagent.model.code.CodeKnowledgeSuggestion;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
+import jakarta.persistence.LockModeType;
 import org.springframework.data.jpa.repository.JpaRepository;
+import org.springframework.data.jpa.repository.Lock;
+import org.springframework.data.jpa.repository.Modifying;
+import org.springframework.data.jpa.repository.Query;
+import org.springframework.data.repository.query.Param;
 import org.springframework.stereotype.Repository;
 
+import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 
 @Repository
 public interface CodeKnowledgeSuggestionRepository extends JpaRepository<CodeKnowledgeSuggestion, String> {
 
+    /**
+     * Row-locking load used by approve/reject. Without it two concurrent bulk
+     * decides both read the same suggestion as PENDING and both materialize a
+     * {@code schema_documentation} row — the duplicate-row bug that
+     * {@code V116__dedupe_schema_documentation.sql} had to clean up. The second
+     * caller now blocks, then sees APPROVED and returns early.
+     */
+    @Lock(LockModeType.PESSIMISTIC_WRITE)
+    @Query("SELECT s FROM CodeKnowledgeSuggestion s WHERE s.id = :id")
+    Optional<CodeKnowledgeSuggestion> findByIdForUpdate(@Param("id") String id);
+
+    /**
+     * Repoints approvals at the surviving row when duplicate schema_documentation
+     * rows are collapsed. {@code applied_doc_id} is a loose reference, not an FK,
+     * so deleting a duplicate would otherwise leave a suggestion pointing at a row
+     * that no longer exists — silently, since nothing enforces it.
+     */
+    @Modifying(flushAutomatically = true)
+    @Query("UPDATE CodeKnowledgeSuggestion s SET s.appliedDocId = :keepId "
+         + "WHERE s.appliedDocId IN :staleIds")
+    int repointAppliedDocId(@Param("keepId") String keepId,
+                            @Param("staleIds") Collection<String> staleIds);
+
     Page<CodeKnowledgeSuggestion> findByConnectionIdAndStatus(
         String connectionId,
         CodeKnowledgeSuggestion.Status status,

backend/src/main/java/com/dbaagent/repository/SchemaDocumentationRepository.java

@@ -21,7 +21,15 @@ List<SchemaDocumentation> findByConnectionIdAndObjectType(
         SchemaDocumentation.DocumentationType objectType
     );
 
-    Optional<SchemaDocumentation> findByConnectionIdAndObjectTypeAndObjectName(
+    /**
+     * Returns a {@link List}, never an {@link Optional} — the logical key is not
+     * unique in data written before {@code V116__dedupe_schema_documentation.sql}
+     * added the constraint, and an {@code Optional} finder throws
+     * {@code IncorrectResultSizeDataAccessException} on a legacy duplicate rather
+     * than letting the caller repair it. Collapse matches with
+     * {@link com.dbaagent.service.SchemaDocumentationDeduplicator}.
+     */
+    List<SchemaDocumentation> findByConnectionIdAndObjectTypeAndObjectName(
         String connectionId,
         SchemaDocumentation.DocumentationType objectType,
         String objectName
@@ -51,12 +59,13 @@ AND TRIM(d.businessTerms) <> ''
         """)
     long countWithBusinessTerms(@Param("connectionId") String connectionId);
 
-    // Upsert support: find existing AI doc to update instead of creating duplicates
-    Optional<SchemaDocumentation> findByConnectionIdAndObjectTypeAndObjectNameAndSource(
+    // Upsert support: find existing doc to update instead of creating duplicates.
+    // List-returning for the same reason as findByConnectionIdAndObjectTypeAndObjectName above.
+    List<SchemaDocumentation> findByConnectionIdAndObjectTypeAndObjectNameAndSource(
         String connectionId, SchemaDocumentation.DocumentationType objectType,
         String objectName, DocumentationSource source);
 
-    Optional<SchemaDocumentation> findByConnectionIdAndObjectTypeAndObjectNameAndParentObjectAndSource(
+    List<SchemaDocumentation> findByConnectionIdAndObjectTypeAndObjectNameAndParentObjectAndSource(
         String connectionId, SchemaDocumentation.DocumentationType objectType,
         String objectName, String parentObject, DocumentationSource source);
 

backend/src/main/java/com/dbaagent/service/SchemaDescriptionService.java

@@ -34,6 +34,7 @@ public class SchemaDescriptionService {
     private final ColumnProfileRepository columnProfileRepo;
     private final InferredTableRelationshipRepository inferredRelationshipRepository;
     private final TrainingService trainingService;
+    private final SchemaDocumentationDeduplicator schemaDocDeduplicator;
     private final ConnectionService connectionService;
     private final DatabaseProviderRegistry providerRegistry;
     private final ObjectMapper objectMapper = new ObjectMapper();
@@ -71,6 +72,7 @@ public SchemaDescriptionService(
             ColumnProfileRepository columnProfileRepo,
             InferredTableRelationshipRepository inferredRelationshipRepository,
             TrainingService trainingService,
+            SchemaDocumentationDeduplicator schemaDocDeduplicator,
             ConnectionService connectionService,
             DatabaseProviderRegistry providerRegistry,
             @Value("${brain.description.ai-concurrency:4}") int aiConcurrency) {
@@ -80,6 +82,7 @@ public SchemaDescriptionService(
         this.columnProfileRepo = columnProfileRepo;
         this.inferredRelationshipRepository = inferredRelationshipRepository;
         this.trainingService = trainingService;
+        this.schemaDocDeduplicator = schemaDocDeduplicator;
         this.connectionService = connectionService;
         this.providerRegistry = providerRegistry;
         this.aiConcurrency = Math.max(1, aiConcurrency);
@@ -417,13 +420,14 @@ private int saveTableDescription(String connectionId, TableDescription desc,
             : desc.getTableName();
 
         // Upsert table-level doc (find existing AI doc or create new)
-        var existingTableDoc = schemaDocRepo
-            .findByConnectionIdAndObjectTypeAndObjectNameAndSource(
+        var existingTableDoc = schemaDocDeduplicator.collapse(
+            schemaDocRepo.findByConnectionIdAndObjectTypeAndObjectNameAndSource(
                 connectionId, DocumentationType.TABLE, objectName,
-                DocumentationSource.AI_GENERATED);
+                DocumentationSource.AI_GENERATED),
+            objectName + " (TABLE, AI_GENERATED)");
         SchemaDocumentation tableDoc;
-        if (existingTableDoc.isPresent()) {
-            tableDoc = existingTableDoc.get();
+        if (existingTableDoc != null) {
+            tableDoc = existingTableDoc;
             tableDoc.setDescription(desc.getTableDescription());
             tableDoc.setBusinessTerms(desc.getBusinessTerms());
             tableDoc.setConfidence(desc.getConfidence());
@@ -445,13 +449,14 @@ private int saveTableDescription(String connectionId, TableDescription desc,
         // Upsert column-level docs
         for (var col : desc.getColumns()) {
             if (col.getDescription() == null || col.getDescription().isBlank()) continue;
-            var existingColDoc = schemaDocRepo
-                .findByConnectionIdAndObjectTypeAndObjectNameAndParentObjectAndSource(
+            var existingColDoc = schemaDocDeduplicator.collapse(
+                schemaDocRepo.findByConnectionIdAndObjectTypeAndObjectNameAndParentObjectAndSource(
                     connectionId, DocumentationType.COLUMN, col.getName(),
-                    objectName, DocumentationSource.AI_GENERATED);
+                    objectName, DocumentationSource.AI_GENERATED),
+                objectName + "." + col.getName() + " (COLUMN, AI_GENERATED)");
             SchemaDocumentation colDoc;
-            if (existingColDoc.isPresent()) {
-                colDoc = existingColDoc.get();
+            if (existingColDoc != null) {
+                colDoc = existingColDoc;
                 colDoc.setDescription(col.getDescription());
                 colDoc.setConfidence(col.getConfidence());
             } else {