feat: classification review UI with bulk-accept guardrails#88
feat: classification review UI with bulk-accept guardrails#88chitcommit merged 3 commits intomainfrom
Conversation
Adds a new POST /api/classification/ai-suggest endpoint that batches
up to 25 unclassified transactions through GPT-4o-mini with structured
JSON output, falling back to keyword matching on any failure.
Library (server/lib/classification-ai.ts):
- classifyBatchWithAI() — per-request OpenAI client (Workers-safe)
- Validates AI-returned codes against the tenant's COA (rejects hallucinations)
- Clamps confidence to [0, 1]
- Always returns one suggestion per input (gap-fills with keyword fallback)
- Respects CF AI Gateway via AI_GATEWAY_ENDPOINT env
Route:
- Only processes transactions without an existing suggested_coa_code
(respects prior human/auto work)
- Writes via L1 suggest (never authoritative coa_code)
- Returns {processed, suggested, aiCount, keywordCount, aiAvailable}
Tests (9 new):
- Empty input, missing key, API error, malformed JSON, missing choices
- Confidence clamping, hallucinated-code rejection, gap-filling
- Batch size guard (MAX_BATCH=25)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Docstring now correctly describes gpt-4o-mini default (was: GPT-4o) - Remove unused `date` field from ClassifiableTransaction interface - ai-suggest only swallows ClassificationError.reconciled_locked; DB/runtime failures now propagate to the shared error handler instead of being silently skipped - `processed` field is now consistent: all branches return txns.length (count of transactions fetched and considered), matching the semantics of the early-return paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Frontend for the trust-path classification workflow added in #86/#87. New: client/src/pages/Classification.tsx - Stats cards (total, classified, reconciled, unclassified + %) - Unclassified queue sorted by date/amount/confidence (ascending or descending) - Per-row actions: accept suggested code (one-click), classify as any COA code (dropdown), reconcile (L3 lock) - Three bulk actions: - Keyword Suggest: runs /api/classification/batch-suggest (L1) - AI Suggest (GPT): runs /api/classification/ai-suggest (L1, 25 at a time) - Accept High-Confidence: writes authoritative coa_code (L2) for rows passing the bulkAcceptCandidates guardrails bulkAcceptCandidates rules (defense in depth): - Confidence >= 0.8 (AI "success" threshold) - suggestedCoaCode !== '9010' (suspense means "needs a human") - |amount| <= $500 (large-dollar → manual review regardless of confidence) New: client/src/hooks/use-classification.ts - TanStack Query hooks scoped by tenantId for automatic cache invalidation on tenant switch - Queries: coa, stats, unclassified, audit trail - Mutations: classify (L2), suggest (L1 manual), reconcile (L3), batch-suggest (L1 keyword), ai-suggest (L1 GPT) Wiring: - /classification route added to App.tsx - Sidebar link added (cfo, accountant, bookkeeper roles) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughA new classification feature is introduced with routing, navigation sidebar updates, client-side React Query hooks for classification workflows, a Classification page component with sorting and bulk acceptance controls, server-side AI-powered batch classification logic using OpenAI, an API route for AI suggestions, and comprehensive test coverage for the AI classifier. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Classification<br/>Page UI
participant Hook as useAiSuggest<br/>Hook
participant API as POST<br/>/ai-suggest
participant OpenAI as OpenAI<br/>API
participant DB as Database
User->>UI: Click "Suggest All"
UI->>Hook: trigger mutation
activate Hook
Hook->>API: POST with unclassified txns
activate API
API->>DB: fetch transactions (limit)
DB-->>API: unclassified data
API->>DB: fetch chart of accounts
DB-->>API: COA list
API->>OpenAI: send batch + COA options<br/>(with model, gateway)
activate OpenAI
OpenAI-->>API: JSON suggestions
deactivate OpenAI
Note over API: validate COA codes<br/>filter valid results<br/>apply fallback
loop For each suggestion
API->>DB: write classification<br/>(with confidence/reason)
end
API-->>Hook: { processed, suggested,<br/>aiCount, keywordCount }
deactivate API
Hook-->>UI: update cache +<br/>mutation result
deactivate Hook
UI->>User: display summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Code Review — Classification Review UI (#88)Overall this is solid work. The trust-path guardrails are well-reasoned, the AI fallback strategy is defensive, and the test coverage on Bug:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 910dd0e9dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function useUnclassifiedTransactions(limit = 50) { | ||
| const tenantId = useTenantId(); | ||
| return useQuery<UnclassifiedTransaction[]>({ | ||
| queryKey: ['/api/classification/unclassified', tenantId, limit], |
There was a problem hiding this comment.
limit in unclassified fetch URL
useUnclassifiedTransactions(limit) never sends limit to the backend because the default query function only requests queryKey[0] as the URL. Here limit is only placed in the query key tuple, so calls like useUnclassifiedTransactions(100) still hit /api/classification/unclassified without ?limit=100 and silently fall back to the server default page size.
Useful? React with 👍 / 👎.
|
|
||
| export function useClassificationAudit(transactionId: string | null) { | ||
| return useQuery<ClassificationAuditEntry[]>({ | ||
| queryKey: ['/api/classification/audit', transactionId], |
There was a problem hiding this comment.
useClassificationAudit is wired to /api/classification/audit even though the route is /api/classification/audit/:transactionId. Because the shared query function only fetches queryKey[0], putting transactionId in the second key slot does not affect the request URL, so this hook will consistently request a non-existent endpoint when used.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new end-to-end UI surface for the trust-path transaction classification workflow, including bulk actions and an AI-backed suggestion path, wiring it into the existing tenant-scoped classification APIs.
Changes:
- Adds
/classificationpage with queue, stats, sorting, and per-transaction classify/reconcile actions. - Adds bulk actions (keyword suggest, AI suggest, accept high-confidence) and tenant-scoped React Query hooks to keep stats/queue in sync.
- Introduces
POST /api/classification/ai-suggestplusserver/lib/classification-ai.ts(OpenAI structured JSON + keyword fallback) with unit tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/routes/classification.ts | Adds AI batch-suggest endpoint that writes L1 suggestions and logs batch activity. |
| server/lib/classification-ai.ts | Implements GPT-4o-mini batch classification with strict code validation + keyword fallback. |
| server/tests/classification-ai.test.ts | Adds unit tests for AI parsing/fallback behavior (currently has TS + mocking issues). |
| client/src/pages/Classification.tsx | New classification queue page with bulk actions and guardrails for bulk accept. |
| client/src/hooks/use-classification.ts | New tenant-scoped queries/mutations for COA + classification workflow. |
| client/src/components/layout/Sidebar.tsx | Adds nav entry for the new Classification page. |
| client/src/App.tsx | Registers the /classification route. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const TX_REPAIR: ClassifiableTransaction = { | ||
| id: 'tx-1', | ||
| description: 'Home Depot #1234 — plumbing supplies', | ||
| amount: '-125.40', | ||
| category: 'Repairs', | ||
| date: '2026-01-15', | ||
| }; |
There was a problem hiding this comment.
The TX_* fixtures include a date property, but ClassifiableTransaction in server/lib/classification-ai.ts does not define date. Because these are object literals typed as ClassifiableTransaction, this will fail TypeScript excess-property checks. Either remove the date fields from the fixtures or extend ClassifiableTransaction to include them (and ensure the production code actually uses it).
| import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
| import { classifyBatchWithAI } from '../lib/classification-ai'; | ||
| import type { ClassifiableTransaction, CoaOption } from '../lib/classification-ai'; | ||
|
|
There was a problem hiding this comment.
vi.mock('openai', ...) is declared after statically importing classifyBatchWithAI. In ESM/Vitest this can prevent the mock from applying before server/lib/classification-ai.ts imports openai, causing the real SDK to be used and the tests to fail/flap. Move the OpenAI mock setup before importing the module under test, or switch to dynamic await import(...) after vi.mock/vi.resetModules() so the mock is guaranteed to intercept the import.
| function handleAcceptAll() { | ||
| const candidates = bulkAcceptCandidates(sortedTxns, coa); | ||
| if (candidates.length === 0) { | ||
| setLastResult('No candidates qualified for bulk accept'); | ||
| return; | ||
| } | ||
| // Fire mutations sequentially to avoid overloading the classify endpoint | ||
| let done = 0; | ||
| for (const tx of candidates) { | ||
| if (!tx.suggestedCoaCode) continue; | ||
| classify.mutate( | ||
| { transactionId: tx.id, coaCode: tx.suggestedCoaCode, reason: 'Bulk accept: high-confidence suggestion' }, | ||
| { | ||
| onSuccess: () => { | ||
| done++; | ||
| if (done === candidates.length) setLastResult(`Bulk accepted ${done} transactions`); | ||
| }, | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
handleAcceptAll says it fires mutations sequentially, but the loop calls classify.mutate(...) repeatedly without awaiting, which will dispatch requests concurrently. This can overload the endpoint and also leaves lastResult unset if any mutation fails (since done only increments on success). Consider using mutateAsync with await (or chaining) and add error handling so the UI reports partial/failed bulk accepts deterministically.
| function handleAcceptAll() { | |
| const candidates = bulkAcceptCandidates(sortedTxns, coa); | |
| if (candidates.length === 0) { | |
| setLastResult('No candidates qualified for bulk accept'); | |
| return; | |
| } | |
| // Fire mutations sequentially to avoid overloading the classify endpoint | |
| let done = 0; | |
| for (const tx of candidates) { | |
| if (!tx.suggestedCoaCode) continue; | |
| classify.mutate( | |
| { transactionId: tx.id, coaCode: tx.suggestedCoaCode, reason: 'Bulk accept: high-confidence suggestion' }, | |
| { | |
| onSuccess: () => { | |
| done++; | |
| if (done === candidates.length) setLastResult(`Bulk accepted ${done} transactions`); | |
| }, | |
| }, | |
| ); | |
| } | |
| async function handleAcceptAll() { | |
| const candidates = bulkAcceptCandidates(sortedTxns, coa); | |
| const actionableCandidates = candidates.filter((tx) => !!tx.suggestedCoaCode); | |
| if (actionableCandidates.length === 0) { | |
| setLastResult('No candidates qualified for bulk accept'); | |
| return; | |
| } | |
| // Fire mutations sequentially to avoid overloading the classify endpoint | |
| let succeeded = 0; | |
| let failed = 0; | |
| for (const tx of actionableCandidates) { | |
| try { | |
| await classify.mutateAsync({ | |
| transactionId: tx.id, | |
| coaCode: tx.suggestedCoaCode!, | |
| reason: 'Bulk accept: high-confidence suggestion', | |
| }); | |
| succeeded++; | |
| } catch { | |
| failed++; | |
| } | |
| } | |
| if (failed === 0) { | |
| setLastResult(`Bulk accepted ${succeeded} transactions`); | |
| return; | |
| } | |
| setLastResult(`Bulk accepted ${succeeded} transactions; ${failed} failed`); |
| return useQuery<ClassificationAuditEntry[]>({ | ||
| queryKey: ['/api/classification/audit', transactionId], | ||
| enabled: !!transactionId, |
There was a problem hiding this comment.
useClassificationAudit's queryKey is not scoped by tenantId, while the rest of the classification hooks are. If a user switches tenants, React Query can serve cached audit data keyed only by transactionId. Include tenantId in the key (and gate enabled on both tenantId + transactionId) to keep caching behavior consistent with the stated tenant-scoping approach.
| return useQuery<ClassificationAuditEntry[]>({ | |
| queryKey: ['/api/classification/audit', transactionId], | |
| enabled: !!transactionId, | |
| const tenantId = useTenantId(); | |
| return useQuery<ClassificationAuditEntry[]>({ | |
| queryKey: ['/api/classification/audit', tenantId, transactionId], | |
| enabled: !!tenantId && !!transactionId, |
Summary
Frontend page for the trust-path classification workflow built in #86 and #87.
What's included
/classification— unclassified transaction queue with per-row classify/reconcile actionsbulkAcceptCandidatesguardrails (defense in depth)>= 0.80coaCode !== '9010'abs(amount) <= $500All three must pass. Large rent payments, tax bills, and ambiguous charges always require explicit click.
Architecture
tenantId— cache invalidates automatically on tenant switch/api/classification/statsand/api/classification/unclassifiedso the UI stays in sync after any action@/lib/queryClient,@/contexts/TenantContext, and Tailwind themeTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit