Skip to content

[Frontend][Inbox] Poll-truth residuals from PR #2567 review: foreground loadingDetail unpinned, count-refresh amplification, summary-generation map growth, aborted negative control #2571

Description

@Chris0Jeky

Non-blocking residuals from the fresh-context review of PR #2567 (closes #2301, #2303, #2304, #2305), tracked here rather than fix-cycled. File references are at that PR's head 744f0cff0.

  1. MEDIUM, unpinned behaviour. refreshTerminalDetails (captureStore.ts ~594) hardcodes trackLoading: false and has two callers: the background poll tick (~756) and batchTriage (~839), which is a foreground user action. A Legacy batch triage that reconciles the open capture's detail no longer raises loadingDetail (the InboxDetailPanel.vue "Refreshing detail" state and the disabled Refresh Detail control). Feedback is not lost because batchBusy stays true for the whole batchTriage body and InboxListPanel.vue renders "Processing" from it, but no spec asserts loadingDetail stays false through batchTriage. Either thread trackLoading through refreshTerminalDetails's options with the poll passing false, or add one assertion in the existing batchTriage specs.
  2. LOW, load characteristic. notifyTriageCountChanged() now fires at most once per tick per newly observed terminal id (~647-658), so a batch whose items finish spread over the 60 s window can issue up to ~20 GET /workspace/home calls where the old code made exactly one. This is what [Frontend][Inbox] Partial batch-triage outcomes do not refresh the workload count #2303 asked for and each call is best-effort, but the endpoint is documented as the heaviest on the surface; consider coalescing per tick.
  3. LOW, growth. latestSummaryGenerationById (~121) is a second per-item Map that grows for the store's lifetime with no eviction on scope change, list replacement or logout; it mirrors the pre-existing latestDetailWriteGenerationById. Correctness is unaffected (monotonic clock; a stale-high entry cannot pin a row). A resetForLogout analogue as in workspaceStore.ts would bound both.
  4. LOW, coverage. [Frontend][Inbox] Successful batch poll recovery leaves a stale list error hiding rows #2305's "aborted" negative control has no spec: superseded and 401/403 are proven not to clear a standing listError, but "poll stopped or aborted mid-flight while a foreground listError stands" is not; the code path (isCurrent() checks !stopped && !controller.signal.aborted) is straightforwardly correct.
  5. Note for future callers. "Same scope" for the listError clear is a caller-side contract: the store clears on any accepted snapshot for the query the poll was constructed with, and only the orchestrator's cancelBatchTriagePolling() on board and archived-history changes (plus unmount) prevents a cross-scope clear. Documented in useInboxOrchestrator.spec.ts.
  6. Cosmetic. The comment above the [Frontend][Inbox] Background batch reconciliation obscures an unrelated open detail #2304 spec (~2364) says "an unrelated open detail" while the test reconciles the same id; the assertion is right, the framing is not.

Refs #2301, #2303, #2304, #2305, PR #2567.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    frontendPrimary implementation impact in Vue/TypeScript UI and client runtime.tech-debtDebt cleanup or deferred engineering quality work with limited product-surface change.testingTest coverage, harnesses, regression prevention, and verification pipeline work.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions