fix: treat missing capture provenance as empty state for manual cards (#680)#754
Conversation
Manual cards have no capture provenance; a 404 from the provenance endpoint is expected, not exceptional. Logging it at error level creates noisy diagnostics that hide real failures. Demote 404s to console.warn so callers that suppress the absence silently don't pollute the console. Fixes #680.
Manual cards have no capture provenance so a 404 from the /provenance endpoint is expected, not exceptional. Catch 404 errors and return null; rethrow all other errors so real failures are not swallowed. Fixes #680.
cardsApi.getCardProvenance now handles 404 internally and returns null, so the duplicate isHttpNotFound check in cardStore is redundant and can be removed. Any non-404 errors will still propagate correctly. Fixes #680.
Replace the plain "No capture provenance available." text with a semantic empty state that clearly communicates the card was created manually. Add data-testid="provenance-empty-state" to enable targeted test assertions. Fixes #680.
- cardsApi: assert 404 returns null; assert non-404 errors rethrow - CardModal: assert data-testid="provenance-empty-state" is shown and no error alert is rendered when provenance is null (manual card) - CardModal: tighten error path assertion to check testid absence Closes #680.
…tructor change AuthController was refactored to inherit AuthenticatedControllerBase in #732, which added a required IUserContext parameter. Three direct instantiations in AuthControllerEdgeCaseTests were not updated at the time. Add a CreateAnonymousUserContext() helper and thread the mock IUserContext into all three construction sites.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Adversarial self-reviewFindings1. Global 404 downgrade in http.ts (acceptable) 2. cardsApi null return vs isHttpNotFound in cardStore (equivalent, confirmed) 3. CardModal conditional rendering (correct) 4. AuthController mock IUserContext (appropriate) No real issues foundThe diff is minimal, correct, and all tests pass (1546 frontend vitest + 30 CardService + 28 CardsApiTests + 28 AuthControllerEdgeCaseTests). |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the handling of card provenance to treat 404 "Not Found" responses as an expected empty state for manually created cards rather than an error. Key changes include updating the cardsApi to return null on 404, adjusting the global HTTP interceptor to log 404s as warnings, and updating the UI and tests to reflect this behavior. Feedback was provided regarding the global downgrade of 404 logs to warnings, as this might reduce visibility for critical missing resources; a per-request configuration is recommended instead.
| if (error.response.status === 404) { | ||
| console.warn('API Not Found:', error.response.data) | ||
| } else { | ||
| console.error('API Error:', error.response.data) | ||
| } |
There was a problem hiding this comment.
Downgrading all 404 responses to console.warn in the global interceptor may reduce visibility for legitimate errors. While this prevents 'noise' for optional resources like card provenance, it also affects 404s for critical resources (e.g., a missing board or user profile) which should likely remain as errors for better observability and monitoring in production environments. Consider allowing individual requests to opt-out of error logging via a custom property in the Axios config instead of applying this change globally.
There was a problem hiding this comment.
Pull request overview
This PR improves the “manual card” UX by treating missing capture provenance as an expected empty state instead of an error, reducing noisy logging and keeping error handling contained at the API layer. It also fixes backend test compilation after the AuthController constructor change from #732.
Changes:
- Frontend:
cardsApi.getCardProvenancereturnsnullon 404; store logic simplified accordingly;CardModaldisplays a dedicated manual-card empty state. - Frontend: global Axios response interceptor logs 404s at
warninstead oferror. - Backend tests: update
AuthControllerinstantiations to includeIUserContextvia a new helper.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/taskdeck-web/src/tests/components/CardModal.spec.ts | Updates modal tests to assert the new provenance empty state and no alert rendering. |
| frontend/taskdeck-web/src/tests/api/cardsApi.spec.ts | Adds tests for getCardProvenance returning null on 404 and rethrowing non-404 errors. |
| frontend/taskdeck-web/src/store/board/cardStore.ts | Simplifies provenance fetch by relying on API-layer 404 handling. |
| frontend/taskdeck-web/src/components/board/CardModal.vue | Updates the provenance fallback UI text and adds a data-testid for the empty state. |
| frontend/taskdeck-web/src/api/http.ts | Downgrades 404 logging from console.error to console.warn in the response interceptor. |
| frontend/taskdeck-web/src/api/cardsApi.ts | Catches 404 for provenance endpoint and returns null instead of throwing. |
| backend/tests/Taskdeck.Api.Tests/AuthControllerEdgeCaseTests.cs | Fixes missing IUserContext constructor arg by adding an anonymous user context helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </p> | ||
| </div> | ||
| <p v-else class="text-xs text-on-surface-variant italic">No capture provenance available.</p> | ||
| <p v-else class="text-xs text-on-surface-variant italic" data-testid="provenance-empty-state">Created manually — no capture provenance.</p> |
There was a problem hiding this comment.
The fallback UI text now specifically says "Created manually", but the modal currently shows the v-else empty state whenever loadingCaptureProvenance is false and captureProvenance is null. On open, props.isOpen watcher awaits fetchCardComments(...) before calling loadCaptureProvenance(), so the UI can briefly render this empty state for captured cards too (until provenance loading starts), which is misleading. Consider showing a neutral loading/placeholder state until provenance has been fetched at least once for the current card (e.g., gate the empty state on loadedCaptureProvenanceCardId === props.card.id, or start loadCaptureProvenance() before awaiting comments / run both in parallel).
| const candidate = e as { response?: { status?: number } } | null | ||
| if (candidate?.response?.status === 404) { | ||
| // Manual cards have no capture provenance — treat absence as empty state, not an error. |
There was a problem hiding this comment.
getCardProvenance currently treats any 404 from the provenance endpoint as an expected empty state. On the backend, the same endpoint returns 404 both when provenance is missing (manual cards) and when the card itself is missing / board mismatch (see CardService.GetCaptureProvenanceAsync), so this can mask real "card not found" errors and lead callers to render the manual-card empty state incorrectly. Consider narrowing the null-return behavior to only the provenance-missing case (e.g., inspect error.response.data.message for the "Capture provenance not found" message, or ideally introduce/consume a more specific error code) and rethrow other 404s.
| const candidate = e as { response?: { status?: number } } | null | |
| if (candidate?.response?.status === 404) { | |
| // Manual cards have no capture provenance — treat absence as empty state, not an error. | |
| const candidate = e as { response?: { status?: number; data?: { message?: string } } } | null | |
| if ( | |
| candidate?.response?.status === 404 && | |
| candidate.response.data?.message === 'Capture provenance not found' | |
| ) { | |
| // Manual cards have no capture provenance — treat only that absence as empty state, not an error. |
…mpty-state on loaded id - cardsApi.getCardProvenance: only return null for 404s whose message starts with "Capture provenance not found"; rethrow card-not-found 404s so they surface as real errors instead of silently showing manual-card empty state (addresses Copilot review comment) - http.ts interceptor: revert global 404 warn downgrade — the API layer now handles the expected absence silently so the interceptor no longer needs special-casing, and downgrading all 404s globally reduces observability for genuine missing-resource errors (addresses Gemini review comment) - CardModal.vue: gate provenance empty-state on loadedCaptureProvenanceCardId === card.id so the "Created manually" message does not flash for capture-pipeline cards while comments are being awaited before provenance fetch starts (addresses Copilot review comment) - cardsApi.spec.ts: update 404 test to use the narrowed message check, add test for card-not-found 404 that must rethrow (1547 tests pass)
Updates STATUS.md, TESTING_GUIDE.md, MANUAL_TEST_CHECKLIST.md, and IMPLEMENTATION_MASTERPLAN.md to reflect the 9 PRs merged in the 2026-04-04 post-adversarial-review hardening wave. - STATUS.md: adds new wave entry for PRs #741–#756 covering product telemetry taxonomy, two bug fixes (#683/#744, #680/#754), six frontend regression test additions, and two backend webhook test additions; updates Last Updated date - TESTING_GUIDE.md: updates frontend count from 1496 to 1592, backend estimate to ~3010+, combined total to ~4600+; marks #710 and #726 as delivered in wave table; adds resolved entries to Key Gaps section; updates telemetry taxonomy note to delivered; appends three new coverage sections for webhook HMAC, SSRF/delivery reliability, and frontend regression wave - MANUAL_TEST_CHECKLIST.md: adds auth-flow toast check (Section A), WIP-limit dedup check (Section B), manual card provenance empty state check (Section B), board header presence label check (Section B), and D2 router auth guard and workspace state section - IMPLEMENTATION_MASTERPLAN.md: adds item 128 for the post-adversarial hardening wave with all 9 issues, fixes, and wave progress update
Summary
http.tsinterceptor now logs 404 responses atwarnlevel instead oferror, so expected absences don't pollute the consolecardsApi.getCardProvenancenow catches 404 and returnsnullinstead of throwing, keeping the error boundary at the API layercardStore.fetchCardProvenancesimplified: removes the redundantisHttpNotFoundguard since the API layer now handles itCardModal.vueshows"Created manually — no capture provenance."withdata-testid="provenance-empty-state"for targetable assertionsAuthControllerEdgeCaseTests.cswhere threeAuthControllerinstantiations were missing theIUserContextargument added in Fix ChangePassword identity bypass (SEC-20) #732Closes #680.
Test plan