fix: address CodeRabbit findings from PR #132

muness · claude · muness · commit 18a4fce25959 · 2026-03-28T20:31:00.000-04:00
- AdminEndpoints: scope reprocess DELETE to documents being reset, not all tenant data
- ReviewEndpoints finalize: merge discovered fields into case_profiles search_text
- ExtractionBackgroundService: persist extraction_attempts for discovered fields (audit trail)
- ExtractionPipelineService: sort discoveredResults by FieldKey for stable serialization order
- Worker.Testing.cs: add RunExtractionPipelineWithDiscoveredForTests exposing full tuple
- CaseProfileText.BuildFinalized: accept optional discoveredFields parameter
- ReviewerDashboard: show confidence badge on discovered fields
- DevPage: refresh queue state after reprocess succeeds

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/.oh/sessions/131-dev.md b/.oh/sessions/131-dev.md
@@ -18,13 +18,57 @@
 - Admin/dev endpoint or migration to requeue existing documents for reprocessing
 
 ## Phase 2: Solution Space
-(filled by Phase 2)
+
+**Selected approach:** Option C — open-ended prompt + schema overlay.
+
+- Change vision prompt to "extract ALL fields + schema fields"
+- Provider marks discovered keys with `is_discovered=true` in metadata
+- `ExtractionPipelineService.RunExtractionPipeline` returns `(schemaFields, discoveredFields)` tuple
+- Discovered fields stored with `is_discovered = true`, `requires_review = false`, don't affect document status
+- `interviewDate` added to all 4 template schemas via M006 migration + SeedTemplatesSql update
+- Frontend shows discovered fields in collapsible "Additional fields" section (read-only with confidence badge)
+- `POST /admin/reprocess` endpoint resets documents to `uploaded` status scoped to eligible docs only
 
 ## Phase 3: Execute
-(filled by Phase 3)
+
+**Files changed:**
+- `DatabaseInitializer.cs` — M005 (is_discovered column) + M006 (interviewDate migration) + SeedTemplatesSql (8 templates updated)
+- `OpenAiVisionProvider.cs` — open-ended prompt, emits discovered fields with is_discovered=true in metadata
+- `ExtractionPipelineService.cs` — returns (schemaFields, discoveredFields) tuple, sorts discovered fields by key
+- `ExtractionBackgroundService.cs` — persists schema and discovered fields separately, includes discovered in case profile
+- `CaseProfileService.cs` — BuildCaseProfileText accepts optional discoveredFields
+- `CaseProfileText.cs` (contracts) — BuildFinalized accepts optional discoveredFields, preserved during finalization
+- `Models.cs` (contracts) — ReviewField gains IsDiscovered = false default parameter
+- `ReviewEndpoints.cs` — queries is_discovered, passes discovered fields to finalize search text
+- `AdminEndpoints.cs` — POST /admin/reprocess scoped to eligible documents only
+- `Worker.Testing.cs` — RunExtractionPipelineForTests returns schema only; new RunExtractionPipelineWithDiscoveredForTests returns both
+- `ReviewerDashboard.tsx` — filters discovered from editable fields, shows collapsible "Additional fields" with confidence badges
+- `DevPage.tsx` — Admin section with reprocess button (Admin role gate, clears state on success)
+- `api.ts` — reprocessDocuments method added
+- `types.ts` — ReviewField gains optional isDiscovered
+
+**Build status:** Clean (0 warnings, 0 errors)
+**Tests:** 29/29 worker unit tests pass, 4/4 tenancy unit tests pass
 
 ## Phase 4: Ship
-(filled by Phase 4)
+
+**PR:** #132
+**Review findings addressed:**
+- Unreachable `"true"` string branch removed from ExtractionPipelineService
+- DevPage reprocess button disabled for non-Admin users
+**CodeRabbit findings addressed:**
+- AdminEndpoints reprocess: scoped DELETE to documents being reset (not all tenant data)
+- ReviewEndpoints finalize: discovered fields now merged into case profile search text
+- ExtractionBackgroundService: discovered fields now also persisted to extraction_attempts
+- ExtractionPipelineService: discovered results sorted by FieldKey for stable ordering
+- Worker.Testing.cs: added RunExtractionPipelineWithDiscoveredForTests exposing full tuple
+- ReviewerDashboard: discovered fields now show confidence badge
+- DevPage: reprocess refreshes queue state after success
+
+**Delivery verification:**
+- `pnpm check` passes
+- All unit tests pass
+- Post-reprocess: documents with interviewDate on form will show it in extracted fields after re-run
 
 ## RNA Tool Friction Log
 <!-- Append entries as you encounter friction with RNA MCP tools. -->
diff --git a/apps/web/src/pages/DevPage.tsx b/apps/web/src/pages/DevPage.tsx
@@ -1341,6 +1341,12 @@ export default function DevPage() {
                   try {
                     const result = await api.reprocessDocuments(auth.accessToken)
                     setReprocessResult(`Queued ${result.queued} document(s) for reprocessing (tenant: ${result.tenantId}).`)
+                    // Clear stale state and refresh so the UI reflects the backend reset
+                    setSelectedReviewId(null)
+                    setReviewDetail(null)
+                    setIntakeStatus(null)
+                    setActiveIntakeId(null)
+                    await refreshQueue(auth.accessToken)
                   } catch (err) {
                     setReprocessError(userMessage(err, 'Reprocess failed.'))
                   } finally {
diff --git a/apps/web/src/pages/ReviewerDashboard.tsx b/apps/web/src/pages/ReviewerDashboard.tsx
@@ -1056,12 +1056,23 @@ function ReviewDetail({
               </button>
               {discoveredOpen ? (
                 <dl className="mt-2 space-y-1.5">
-                  {discoveredFields.map((f) => (
-                    <div key={f.fieldKey} className="rounded border border-slate-200 bg-slate-50 px-3 py-2">
-                      <dt className="text-xs font-medium text-slate-600">{f.fieldKey}</dt>
-                      <dd className="mt-0.5 text-xs text-slate-800 break-words">{f.value || '—'}</dd>
-                    </div>
-                  ))}
+                  {discoveredFields.map((f) => {
+                    const tier = confidenceTone(f.confidence)
+                    return (
+                      <div key={f.fieldKey} className="rounded border border-slate-200 bg-slate-50 px-3 py-2">
+                        <div className="flex items-center justify-between gap-2">
+                          <dt className="text-xs font-medium text-slate-600">{f.fieldKey}</dt>
+                          <span
+                            className={`shrink-0 rounded-full border px-2 py-0.5 text-xs font-medium ${tier.badgeClass}`}
+                            aria-label={`${confidencePercent(f.confidence)} confidence`}
+                          >
+                            {confidencePercent(f.confidence)}
+                          </span>
+                        </div>
+                        <dd className="mt-0.5 text-xs text-slate-800 break-words">{f.value || '—'}</dd>
+                      </div>
+                    )
+                  })}
                 </dl>
               ) : null}
             </div>
diff --git a/src/BuildingBlocks/Northwoods.Contracts/CaseProfileText.cs b/src/BuildingBlocks/Northwoods.Contracts/CaseProfileText.cs
@@ -10,14 +10,15 @@ namespace Northwoods.Contracts;
 public static class CaseProfileText
 {
     /// <summary>
-    /// Builds search text incorporating finalized field values, OCR segments, and reviewer note.
-    /// Used when a reviewer finalizes a document so the case profile reflects corrections.
+    /// Builds search text incorporating finalized field values, OCR segments, reviewer note,
+    /// and any discovered (non-schema) fields so they remain searchable after finalization.
     /// </summary>
     public static string BuildFinalized(
         string templateId,
         IReadOnlyList<ConfidenceField> fields,
         IEnumerable<(string FieldKey, string RawValue)> ocrSegments,
-        string? reviewerNote)
+        string? reviewerNote,
+        IEnumerable<(string FieldKey, string Value)>? discoveredFields = null)
     {
         var fieldPairs = fields
             .Select(f => $"{f.FieldKey}: {f.Value}")
@@ -38,6 +39,17 @@ public static string BuildFinalized(
         if (!string.IsNullOrWhiteSpace(reviewerNote))
             result += $"; reviewer_note={reviewerNote}";
 
+        if (discoveredFields is not null)
+        {
+            var discoveredPairs = discoveredFields
+                .Select(d => $"{d.FieldKey}: {d.Value}")
+                .Where(v => !string.IsNullOrWhiteSpace(v))
+                .ToArray();
+
+            if (discoveredPairs.Length > 0)
+                result += $"; discovered={string.Join(" | ", discoveredPairs)}";
+        }
+
         return result;
     }
 }
diff --git a/src/Services/Northwoods.Api/Endpoints/AdminEndpoints.cs b/src/Services/Northwoods.Api/Endpoints/AdminEndpoints.cs
@@ -72,26 +72,33 @@ del_docs AS (
 
             var queued = await session.Connection.ExecuteScalarAsync<int>(
                 """
-                WITH clear_attempts AS (
+                WITH target_docs AS (
+                    SELECT id FROM documents
+                    WHERE tenant_id = @TenantId
+                      AND status IN ('finalized', 'completed', 'review_ready', 'failed')
+                ),
+                clear_attempts AS (
                     DELETE FROM extraction_attempts
                     WHERE tenant_id = @TenantId
+                      AND document_id IN (SELECT id FROM target_docs)
                     RETURNING 1
                 ),
                 clear_fields AS (
                     DELETE FROM extracted_fields
                     WHERE tenant_id = @TenantId
+                      AND document_id IN (SELECT id FROM target_docs)
                     RETURNING 1
                 ),
                 clear_profiles AS (
                     DELETE FROM case_profiles
                     WHERE tenant_id = @TenantId
+                      AND document_id IN (SELECT id FROM target_docs)
                     RETURNING 1
                 ),
                 reset_docs AS (
                     UPDATE documents
                     SET status = 'uploaded', updated_at = now()
-                    WHERE tenant_id = @TenantId
-                      AND status IN ('finalized', 'completed', 'review_ready', 'failed')
+                    WHERE id IN (SELECT id FROM target_docs)
                     RETURNING 1
                 )
                 SELECT COUNT(*)::int FROM reset_docs
diff --git a/src/Services/Northwoods.Api/Endpoints/ReviewEndpoints.cs b/src/Services/Northwoods.Api/Endpoints/ReviewEndpoints.cs
@@ -297,11 +297,21 @@ INSERT INTO audit_events (document_id, tenant_id, event_type, actor_id, details)
                     new { DocId = id, TenantId = authContext.TenantId },
                     session.Transaction)).ToList();
 
+                var discoveredFieldRows = (await session.Connection.QueryAsync<(string field_key, string extracted_value)>(
+                    """
+                    SELECT field_key, COALESCE(corrected_value, extracted_value) AS extracted_value
+                    FROM extracted_fields
+                    WHERE document_id = @DocId AND tenant_id = @TenantId AND COALESCE(is_discovered, false) = true
+                    """,
+                    new { DocId = id, TenantId = authContext.TenantId },
+                    session.Transaction)).ToList();
+
                 var searchText = CaseProfileText.BuildFinalized(
                     doc.template_id,
                     request.Fields,
                     ocrSegments.Select(o => (o.field_key, o.raw_value)),
-                    request.ReviewerNote);
+                    request.ReviewerNote,
+                    discoveredFieldRows.Select(d => (d.field_key, d.extracted_value)));
 
                 string? embeddingLiteral = null;
                 var apiKey = openAiApiKey;
diff --git a/src/Workers/Extraction.Worker/ExtractionBackgroundService.cs b/src/Workers/Extraction.Worker/ExtractionBackgroundService.cs
@@ -275,6 +275,11 @@ ON CONFLICT (document_id, field_key) DO UPDATE
                             Confidence = result.SystemConfidence
                         },
                         tx);
+
+                    if (canPersistAttempts)
+                    {
+                        await ExtractionPipelineService.PersistExtractionAttempts(conn, tx, docId, tenantId, extractionRunId, result, ct);
+                    }
                 }
 
                 if (canPersistCaseProfiles)
diff --git a/src/Workers/Extraction.Worker/Services/ExtractionPipelineService.cs b/src/Workers/Extraction.Worker/Services/ExtractionPipelineService.cs
@@ -65,6 +65,7 @@ internal static class ExtractionPipelineService
             discoveredResults.Add(FieldConsensus.Resolve(key, attempts));
         }
 
+        discoveredResults.Sort((a, b) => string.Compare(a.FieldKey, b.FieldKey, StringComparison.OrdinalIgnoreCase));
         return (schemaResults, discoveredResults);
     }
 
diff --git a/src/Workers/Extraction.Worker/Worker.Testing.cs b/src/Workers/Extraction.Worker/Worker.Testing.cs
@@ -20,6 +20,7 @@ public sealed partial class Worker
     internal static FieldExtractionResult ResolveConsensusForTests(string key, IReadOnlyList<ExtractionCandidate> attempts)
         => FieldConsensus.Resolve(key, attempts);
 
+    /// <summary>Returns schema fields only. Use RunExtractionPipelineWithDiscoveredForTests to get both.</summary>
     internal static async Task<IReadOnlyList<FieldExtractionResult>> RunExtractionPipelineForTests(
         ExtractionContext context,
         IReadOnlyList<string> fieldKeys,
@@ -30,6 +31,14 @@ internal static async Task<IReadOnlyList<FieldExtractionResult>> RunExtractionPi
         return schemaFields;
     }
 
+    /// <summary>Returns both schema fields and discovered fields.</summary>
+    internal static Task<(IReadOnlyList<FieldExtractionResult> SchemaFields, IReadOnlyList<FieldExtractionResult> DiscoveredFields)> RunExtractionPipelineWithDiscoveredForTests(
+        ExtractionContext context,
+        IReadOnlyList<string> fieldKeys,
+        IReadOnlyList<IExtractionProvider> providers,
+        CancellationToken cancellationToken)
+        => ExtractionPipelineService.RunExtractionPipeline(context, fieldKeys, providers, cancellationToken);
+
     /// <summary>
     /// Determines the document status based on ADR 005 confidence tiers.
     /// Returns (status, autoAccepted, requiresAttention).

Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,7 @@ internal static class ExtractionPipelineService`
`65`	`65`	`discoveredResults.Add(FieldConsensus.Resolve(key, attempts));`
`66`	`66`	`}`
`67`	`67`
	`68`	`+ discoveredResults.Sort((a, b) => string.Compare(a.FieldKey, b.FieldKey, StringComparison.OrdinalIgnoreCase));`
`68`	`69`	`return (schemaResults, discoveredResults);`
`69`	`70`	`}`
`70`	`71`