Add batch triage and suggestion editing for Inbox artifacts

[Chris0Jeky]

Summary

• Backend: Add POST /api/capture/items/batch-triage endpoint accepting multiple item IDs with per-item actions (triage, ignore, cancel). Returns 200/207/422 based on success/partial/total failure. Add PUT /api/capture/items/{id}/suggestion for editing capture text before triage with state-transition guards.
• Frontend: Add multi-select checkboxes with select-all toggle in InboxView item list. Add batch action bar (Triage/Ignore/Cancel selected, Clear). Add inline suggestion text editing with save/cancel before triage. New store actions: batchTriage, updateSuggestion.
• Validation: Batch size limit (50), duplicate ID rejection, invalid action rejection, per-item authorization, state-transition enforcement for suggestion edits.

Closes #220

Test plan

• Backend unit tests: batch validation (empty, duplicates, invalid actions, oversized), partial failure handling, ignore action, suggestion text editing with authorization and state-transition guards (8 new tests)
• Backend API integration tests: unauth checks for new endpoints, batch triage success/partial/empty, suggestion editing with ownership and conflict guards (7 new tests)
• Frontend captureApi tests: batch triage POST, suggestion PUT (2 new tests)
• Frontend captureStore tests: batch dispatch, partial failure toasts, batchBusy tracking, suggestion caching, actionBusyItemId tracking, error handling (7 new tests)
• Frontend InboxView tests: checkbox rendering, select-all, batch bar visibility, batch action dispatch, selection clearing, edit button, editing save/cancel (7 new tests)
• All existing tests pass (backend 31 CaptureService + 25 CaptureApi; frontend 1377 total)

[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]

Adversarial Self-Review

Findings

1. Sequential batch processing (no parallelism risk but N+1 DB concern)
Each batch item triggers a separate GetByIdAsync + potential SaveChangesAsync. For a 50-item batch, this means up to 100+ DB round-trips. Not a correctness issue, but a performance concern for large batches. The current design is safe because each item is processed independently with its own error boundary -- no partial transaction rollback needed. Acceptable for v1 since batches are user-initiated and rate-limited.

2. Missing text length validation on UpdateSuggestionAsync
The UpdateCaptureSuggestionDto.Text is validated for emptiness but not against CaptureRequestContract.MaxRawTextLength (20,000 chars). A malicious client could submit arbitrarily long text. This should be fixed.

3. Batch actions share a single SaveChangesAsync per item
Each item's triage/ignore/cancel calls SaveChangesAsync individually. If item N fails after item N-1 succeeded, item N-1's changes are already persisted. This is by design (partial success), but worth noting: there's no rollback for previously succeeded items in the batch. The 207 status code correctly communicates this.

4. Frontend selectedIds reactivity with Set
Vue's reactivity system doesn't deeply track Set mutations. The implementation correctly creates a new Set on each toggle (const next = new Set(...)) to ensure reactivity. Good.

5. canMutateSelection check on edit button
The "Edit Text" button is gated by canMutateSelection which only allows New and Failed statuses. However, the backend UpdateSuggestionAsync also allows Triaged status. Minor UX gap -- users cannot edit triaged items from the UI even though the backend supports it. Low risk.

6. No CSRF concern
The batch endpoint requires JWT auth (same as all other endpoints). Rate limiting is applied via CaptureWritePerUser. No new attack surface.

Actions Taken

• Will fix item Add testing readiness review and action plan #2 (text length validation) and item Review recent changes and documentation #5 (allow Triaged items to be edited in UI).

[Chris0Jeky]

Self-review fixes applied (197c7c8)

1. Text length validation: Added MaxRawTextLength (20,000) and MaxTitleHintLength (240) validation to UpdateSuggestionAsync. Prevents arbitrarily long text from being submitted.
2. Edit button status scope: Added canEditSuggestion function that allows New, Failed, AND Triaged statuses (matching the backend). Previously the UI only showed the edit button for New/Failed.

All backend (31 CaptureService + 25 CaptureApi) and frontend (1377) tests pass after these fixes.

[gemini-code-assist]

Code Review

This pull request introduces batch triage capabilities and the ability to edit capture suggestions before triage. Key changes include new API endpoints for batch operations and suggestion updates, corresponding service layer logic in CaptureService, and a new batch selection UI in the frontend InboxView. I have identified two areas for improvement: the batch processing logic in the backend currently performs sequential database saves which could be optimized to a single transaction or bulk update, and the suggestion editing UI should pre-populate the title hint from the existing item data to improve the user experience.

[gemini-code-assist]

The BatchTriageAsync method processes items sequentially and performs a database save (SaveChangesAsync) for every item in the batch. For a batch size of up to 50, this results in significant overhead due to multiple database roundtrips and sequential execution. It is recommended to refactor the logic to perform updates in memory and call SaveChangesAsync once at the end of the batch, or at least use a single transaction to batch the operations.

[gemini-code-assist]

The startEditSuggestion method should initialize editedTitleHint with the existing value from the selected item instead of an empty string. This ensures that users can see and modify the current hint.

  editedTitleHint.value = selectedItem.value.titleHint ?? ''

[Chris0Jeky]

Adversarial Code Review -- PR #607 (Batch Triage Inbox)

1. DATA LOSS BUG: UpdateSuggestionAsync drops ClientCreatedAt [Severity: HIGH]

File: backend/src/Taskdeck.Application/Services/CaptureService.cs, UpdateSuggestionAsync method.

The updated payload constructor passes null as the 4th positional argument (ClientCreatedAt), silently discarding whatever value was originally stored:

var updatedPayload = new CapturePayloadV1(
    currentPayload.Version,
    currentPayload.Source,
    dto.Text,
    null,                                    // BUG: drops currentPayload.ClientCreatedAt
    dto.TitleHint ?? currentPayload.TitleHint,
    currentPayload.ExternalRef,
    currentPayload.Provenance);

CapturePayloadV1 has ClientCreatedAt as an optional parameter with default null, so this compiles fine but loses data. Should be currentPayload.ClientCreatedAt instead of null.

---

2. NO TRANSACTION AROUND BATCH -- PARTIAL MUTATIONS ARE SILENTLY COMMITTED [Severity: MEDIUM-HIGH]

File: backend/src/Taskdeck.Application/Services/CaptureService.cs, BatchTriageAsync method.

Each item in the batch calls CancelInternalAsync or ExecuteBatchItemTriageAsync, and each of those calls SaveChangesAsync individually. There is no wrapping transaction. If item 3 of 5 fails mid-batch:

• Items 1-2 are already persisted to the database
• Items 3-5 are not processed (or fail)
• The response correctly reports partial success (207), but the user has no way to retry just the failed items without re-submitting the entire batch

This is acceptable if documented as intended behavior, but it means the batch endpoint is not atomic. If a transient DB error occurs partway through, the user is left in a half-processed state with no built-in retry mechanism.

Additionally, each sub-operation does its own GetByIdAsync + SaveChangesAsync round-trip -- for a batch of 50, that is 50-100 DB round-trips. Consider loading all items in a single query upfront.

---

3. STALE SELECTION STATE AFTER BOARD/FILTER CHANGE [Severity: MEDIUM]

File: frontend/taskdeck-web/src/views/InboxView.vue

selectedIds is never cleared when:

• activeBoardId changes (line ~538-542 resets selectedItemId and activeItemIndex but not selectedIds)
• The items list changes due to a refresh/filter (the watch on items at line ~514 does not touch selectedIds)

This means: select 3 items in Board A, switch to Board B, and the batch action bar still shows "3 selected" with stale IDs that no longer appear in the list. Clicking "Triage" would send IDs that belong to Board A, which will either fail (if authorization is board-scoped) or succeed unexpectedly (if it is user-scoped only), triaging items the user did not intend to act on in the current context.

Fix: Clear selectedIds in the watch(activeBoardId) handler and prune stale IDs in the watch(items) handler.

---

4. ignore AND cancel ARE IDENTICAL -- CONFUSING API SURFACE [Severity: LOW-MEDIUM]

File: backend/src/Taskdeck.Application/Services/CaptureService.cs, BatchTriageAsync switch statement.

Both "ignore" and "cancel" map to the exact same CancelInternalAsync call. This is a confusing API contract -- callers have two names for the same operation with no behavioral difference. The frontend exposes both as separate buttons in the batch action bar ("Ignore" and "Cancel"). Either:

• Document that they are aliases and pick one canonical name, or
• Give them distinct semantics (e.g., "ignore" could set a different status/flag than "cancel")

---

5. USER INPUT REFLECTED IN ERROR MESSAGES [Severity: LOW-MEDIUM]

File: backend/src/Taskdeck.Application/Services/CaptureService.cs, validation block.

return Result.Failure<BatchTriageResultDto>(ErrorCodes.ValidationError,
    $"Invalid action(s): {string.Join(", ", invalidActions.Select(i => i.Action))}. Valid actions: triage, ignore, cancel");

The raw user-supplied Action string is reflected directly into the error message. While this is a validation error (not an HTML page), if these error messages are ever logged or displayed in a context that interprets markup, this could be an injection vector. Consider sanitizing or truncating the reflected values.

---

6. NO AUTHORIZATION TEST FOR CROSS-USER BATCH TRIAGE [Severity: MEDIUM]

Files: backend/tests/Taskdeck.Application.Tests/Services/CaptureServiceTests.cs, backend/tests/Taskdeck.Api.Tests/CaptureApiTests.cs

There is no test verifying that a batch containing another user's item IDs returns per-item Forbidden errors. The authorization check exists in CancelInternalAsync and EnqueueTriageAsync (they check item.UserId != userId), but there is no test proving:

1. A batch with a mix of own + foreign items returns 207 with correct per-item errors
2. A batch with all foreign items returns 422

The single-item UpdateSuggestion has a cross-user forbidden test, but the batch endpoint does not.

---

7. MISSING VALIDATION: Guid.Empty ITEM IDs IN BATCH [Severity: LOW]

File: backend/src/Taskdeck.Application/Services/CaptureService.cs, BatchTriageAsync.

The batch validates for duplicate IDs but does not check for Guid.Empty item IDs. Sending { itemId: "00000000-0000-0000-0000-000000000000", action: "triage" } will pass validation and proceed to GetByIdAsync, which will either return null (triggering a per-item NotFound) or potentially match an unintended record if one exists with that ID. Not a security hole per se, but an early-reject would be cleaner.

---

8. MISSING NULL CHECK ON Action STRING [Severity: LOW]

File: backend/src/Taskdeck.Application/DTOs/CaptureDtos.cs

public record BatchTriageItemActionDto(
    Guid ItemId,
    string Action);

Action is a non-nullable string, but JSON deserialization can still produce null if the caller sends { "itemId": "...", "action": null }. The ValidBatchActions.Contains(i.Action) call at the validation step would throw ArgumentNullException because HashSet with StringComparer.OrdinalIgnoreCase does not accept null keys. This would surface as a 500 instead of a 400.

---

9. TEST GAP: 207 STATUS CODE NOT TESTED AT API INTEGRATION LEVEL FOR ALL-FAIL CASE [Severity: LOW]

File: backend/tests/Taskdeck.Api.Tests/CaptureApiTests.cs

BatchTriage_ShouldReturnPartialSuccess_WhenSomeItemsFail tests the 207 case. But there is no integration test for the all-fail case (expected: 422 UnprocessableEntity). The unit test covers it conceptually but the API-level status code mapping is not verified.

---

Summary

#	Finding	Severity
1	ClientCreatedAt silently dropped in UpdateSuggestionAsync	HIGH
2	No transaction wrapping batch; N round-trips per batch	MEDIUM-HIGH
3	Stale selectedIds after board/filter switch	MEDIUM
4	ignore and cancel are identical operations	LOW-MEDIUM
5	User input reflected in error messages	LOW-MEDIUM
6	No cross-user authorization test for batch endpoint	MEDIUM
7	No Guid.Empty validation on batch item IDs	LOW
8	Null Action string causes 500 instead of 400	LOW
9	Missing all-fail 422 API integration test	LOW

Items 1, 3, and 6 should be addressed before merge. Item 2 is an architectural decision worth documenting. Items 4-5, 7-8, 9 are improvements that could be follow-ups.