Show meaningful error messages for failed capture triage

[Chris0Jeky]

Summary

• Add errorMessage field to backend CaptureItemDto and map it from LlmRequest.ErrorMessage so the API returns failure details
• Add errorMessage to the frontend CaptureItem type
• Display an error banner in the InboxView detail panel when a capture has Failed status, showing the error message and a user-friendly hint ("You can edit the text and retry, or ignore this capture if it is no longer needed.")
• Change the triage button label to "Retry Triage" for failed items

Closes #615

Test plan

• Backend builds clean (0 errors)
• All 1588 backend tests pass
• Frontend typecheck passes
• All 1444 frontend unit tests pass
• Manual: create a capture, force a triage failure (e.g. via mock provider error), verify the error banner appears in the detail panel with the error message and hint
• Manual: verify "Retry Triage" button label appears and clicking it re-submits the capture
• Manual: verify "Edit Text" button still works for failed items, allowing text correction before retry

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Self-review

Error message sanitization

Verified safe. The backend already runs SensitiveDataRedactor.SanitizeLlmFailureMessage() in LlmQueueToProposalWorker before calling item.MarkAsFailed(safeErrorMessage). The stored ErrorMessage is pre-sanitized -- no raw API keys, tokens, or internal URLs leak to the frontend. Vue {{ }} interpolation auto-escapes HTML, preventing XSS.

Retry idempotency

Verified safe. EnqueueTriageAsync calls item.ResetForRetry() (clears ErrorMessage, resets status to Pending) then MarkAsProcessing(). The domain entity enforces state transitions -- ResetForRetry() only works from Failed status, and MarkAsProcessing() rejects already-processing items. Multiple rapid clicks are guarded by the existing actionBusyItemId disable check on the button.

Error banner visibility

The banner only renders when status === Failed AND errorMessage is truthy. If a capture fails but has no error message (edge case -- MarkAsFailed requires non-empty message), the banner is hidden and the "Failed" status chip + "Retry Triage" button still appear, so the user is not left without recourse.

DTO field ordering

ErrorMessage is inserted before Provenance (both optional/defaulted) in the record constructor. Existing callers that use positional construction (the MapToDetailDto method) are updated. Named-parameter callers are unaffected since both new and old parameters have defaults.

No issues found requiring changes.

[gemini-code-assist]

Code Review

This pull request introduces error reporting for failed triage items by adding an ErrorMessage property to the backend DTOs and the frontend CaptureItem interface. The UI now includes a 'Retry Triage' button and an error banner that displays when a capture fails. Feedback was provided to ensure the error banner remains visible even if a specific error message is missing, so that users always receive guidance on how to proceed when a triage fails.

[gemini-code-assist]

The error banner is currently hidden if selectedItem.errorMessage is null or empty, even if the status is Failed. This means users won't see the helpful hint about editing and retrying if the backend fails to provide a specific error message (e.g., due to a generic timeout or unhandled exception).

It is better to show the banner whenever the status is Failed, and only conditionally render the specific error message paragraph.

            v-if="selectedItem.status === 6 || selectedItem.status === 'Failed'"
            class="td-inbox-detail__error-banner"
            data-testid="capture-error-banner"
            role="alert"
          >
            <p class="td-inbox-detail__error-title">Triage failed</p>
            <p v-if="selectedItem.errorMessage" class="td-inbox-detail__error-message">{{ selectedItem.errorMessage }}</p>
            <p class="td-inbox-detail__error-hint">
              You can edit the text and retry, or ignore this capture if it is no longer needed.
            </p>
          </div>

[Chris0Jeky]

Adversarial Code Review -- PR #639

Security

[LOW] Raw error messages exposed to the frontend without length cap

LlmRequest.ErrorMessage is stored with a DB column max of 1000 characters (from the migration), and SensitiveDataRedactor.SanitizeLlmFailureMessage does redact known sensitive patterns (API keys, bearer tokens, payload content) before persisting. This is adequate for the LLM worker path (LlmQueueToProposalWorker.cs:440).

However, there is no max-length enforcement in LlmRequest.MarkAsFailed() itself (compare with OutboundWebhookDelivery which has NormalizeErrorMessage that truncates to 1000 chars). If a future caller bypasses SensitiveDataRedactor and passes a raw exception message directly to MarkAsFailed, it could store and surface verbose internal details. This is not introduced by this PR but is now more visible since the message reaches the UI.

Recommendation: Consider adding a truncation guard in LlmRequest.MarkAsFailed() similar to OutboundWebhookDelivery.NormalizeErrorMessage, as defense-in-depth.

Correctness

[OK] ErrorMessage field is properly nullable. The DTO parameter string? ErrorMessage = null is nullable with a default. The LlmRequest.ErrorMessage domain property is string?. The TypeScript type is errorMessage?: string | null. All consistent.

[OK] Status check handles both numeric and string values. The template condition selectedItem.status === 6 || selectedItem.status === 'Failed' follows the same dual-check pattern used consistently throughout InboxView.vue (e.g., statusLabel, statusChipClass, canMutateSelection). This is correct.

[OK] DTO positional record parameter ordering. Adding ErrorMessage before Provenance in the positional record is safe -- MapToDetailDto passes all arguments positionally and the diff correctly inserts item.ErrorMessage in the right position (line 495 in CaptureService.cs).

UX

[OK] No XSS risk. The error message is rendered via {{ selectedItem.errorMessage }} (text interpolation), not v-html. Vue's text interpolation auto-escapes HTML entities, so this is safe.

[MINOR] "Retry Triage" button label appears but the button handler is unchanged. The triageButtonLabel function now returns 'Retry Triage' for failed items, and canTriageSelection (aliased to canMutateSelection) already permits status 6 / 'Failed'. The triageSelected handler calls captureStore.enqueueTriage, which hits EnqueueTriageAsync in the backend. That method already handles RequestStatus.Failed by calling ResetForRetry() then MarkAsProcessing() (CaptureService.cs:260-265). So the retry flow is functionally complete. Good.

[MINOR] Error banner only shows when errorMessage is truthy. The v-if requires both Failed status AND a non-empty errorMessage. If a capture fails but errorMessage is somehow null/empty (e.g., a code path that doesn't set it), the banner silently hides. This is arguably correct (no message = nothing useful to show), but worth noting -- the user would see a Failed status chip with no explanation.

Contract / API Compatibility

[LOW] Adding ErrorMessage to CaptureItemDto is additive and non-breaking. JSON serialization of a new nullable field defaults to either null or omission, which existing API consumers will ignore per standard JSON deserialization behavior. No breaking change.

Test Gaps

[MEDIUM] No unit tests for the error banner display. The PR adds a new UI element (data-testid="capture-error-banner") and a new button label ('Retry Triage') but includes zero frontend tests covering:

• Error banner renders when status is Failed and errorMessage is present
• Error banner does NOT render when status is Failed but errorMessage is null
• Error banner does NOT render for non-Failed statuses
• "Retry Triage" label appears for Failed items
• The error message text content matches the item's errorMessage

The InboxView.spec.ts file has no coverage for these scenarios. Per CLAUDE.md Definition of Done: "Behavior changes ship with tests."

[MEDIUM] No backend tests for ErrorMessage mapping. The MapToDetailDto change passes item.ErrorMessage through to the DTO, but there are no tests in CaptureService tests verifying that a failed item's error message appears in the returned DTO.

Summary

Severity	Finding
MEDIUM	No frontend tests for error banner rendering or "Retry Triage" label
MEDIUM	No backend tests for ErrorMessage mapping in CaptureItemDto
LOW	No truncation guard in LlmRequest.MarkAsFailed() (defense-in-depth)
LOW	Additive DTO change is non-breaking (just noting for completeness)
OK	No XSS -- uses text interpolation
OK	Retry flow is functionally complete end-to-end
OK	Status checks handle both numeric and string values correctly

The two MEDIUM items (missing tests) should be addressed before merge to satisfy the project's Definition of Done.