fix(chat): gate batch comment submit on gateway connection

[lucasmokwa]

Problem / Motivation

The chat composer is fully gated on the gateway connection — Send/Optimize disable with !connected, offlineProps() sets the offline tooltip/aria state, and ChatPage.send() bails on !connected. The batch comment submit affordances have none of this wiring:

• File panel "Submit All" (CommentList, CommentOverlay.tsx): no disabled prop at all. Clicking while offline composes the batch, unconditionally clears the pending comments (setComments([]) in MarkdownPanel's submitAllComments), and send() silently drops the message — the user's comments are destroyed with no error.
• Artifact panel "Submit" (SubmitBar, ArtifactPanel.tsx): disabled bound only to the 400ms submitting spinner flag. Clicking while offline flashes the fake spinner and drops the batch.
• ChatPage.submitComments also dispatches switchSlot(target) before send() bails, so an offline submit can switch the active session with nothing sent.

Why it matters

Silent data loss of user-authored content: a user who wrote several inline review comments and clicks Submit All during a gateway drop loses all of them with no error and no recovery path. This is exactly the offline draft-loss class the composer's send() guard already protects against (its comment names it) — these three entry points bypass that protection's UI half.

What changed (motivation → approach → change)

Symptom: batch submits fire while disconnected. Root cause: the connected flag never reaches the panels' submit affordances, and the submit handlers compose-and-clear before the refused send. Change: mirror ChatInput's Send gating end to end —

• Plumb connected (optional prop, default true) from ChatPage (useConnected()) → SidePanel → TabBody/FileTabBody → MarkdownPanel/ArtifactPanel → CommentList/SubmitBar.
• Disable both submit buttons with disabled={!connected} + the shared offlineProps(connected, 'submit comments', …) affordance (same tooltip/aria pattern as the composer).
• Guard submitAllComments / submitToChat / submitComments on connected as the behavioral backstop, so pending comments are never composed + cleared on a refused send, and an offline submit can no longer switch the active session.

Prop-with-default (rather than calling useConnected() in the panels) follows the existing convention — presentational panels are rendered in tests without a Redux provider, and non-chat embeddings (no onSubmitComments) hide the affordance anyway.

Tests

• website/src/test/CommentListOffline.test.tsx (new, 4 tests): offline disables Submit All with the aria/title affordance; offline click does not fire onSubmitAll; connected default and connected=true carry no offline affordance and submit normally.
• website/src/test/ArtifactPanelCoverage.test.tsx ("offline gating (submit-to-chat)" describe, 3 tests): offline disables Submit with the affordance; offline click does not fire onSubmitComments; omitted connected defaults to enabled and submits.

Manual verification

Rendered the real CommentList in both states via a throwaway Vite harness (not committed) and verified in the live DOM: connected button disabled=false, aria-disabled=false; disconnected button disabled=true, aria-disabled=true, title="Gateway offline — reconnect to submit comments", aria-label="Submit All disabled — gateway offline". Screenshot below is from that run.

Screenshots / video

File panel comment list, connected (unchanged) vs disconnected (Submit All disabled + offline tooltip/aria):

The artifact panel's Submit button gets the identical SendBtn + offlineProps treatment (asserted by the ArtifactPanelCoverage tests above).

Related Issues

no linked issue: found and fixed while porting an internal fix; no tracking issue exists for it.

Pattern harvest

Rule candidate: review-prompt
Pattern: an action path that clears user-authored state (pending comments, drafts) before an unconfirmed send must carry the same connected gating as the composer — button disabled + offlineProps + handler bail, all three.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 787ae4dd4da74dec6c8bbca5d738d03bec912241 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All consumers verified: the only onSubmitComments producers are ChatPage → SidePanel, both wired; temp-screenshots/ is an established repo convention; offlineProps is the existing composer pattern. Emitting the review.

Design-Verdict: PASS

Real silent-data-loss bug, fixed at both the UI affordance and the handler, using the codebase's established offlineProps/prop-with-default convention — sound and proportionate.

Watch

• The guard is fail-open: connected defaults true in every panel, and nothing type-couples it to onSubmitComments. A future call site that passes the callback but forgets the flag silently reintroduces the exact clear-then-drop bug — worth a note where the props are declared, or pairing them in one prop.

Suggestions

• The deeper root cause is "clear user state before an unacknowledged send." Having onSubmitComments return accepted/refused and clearing only on acceptance would protect against any future refusal path, not just offline; reasonable as a follow-up, not this PR.

[DESIGN-REVIEWED] 787ae4d

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

UX-level review of 787ae4dd4da74dec6c8bbca5d738d03bec912241 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

This is a tight, pattern-conformant fix. The new offline gating exactly mirrors the composer's established offlineProps + disabled + handler-bail pattern (ChatInput.tsx:3809, SessionTabStrip, ChatSidebar), SendBtn already renders a clear disabled state (disabled:opacity-30 disabled:cursor-not-allowed), the tooltip asserts the state and offers the action ("Gateway offline — reconnect to submit comments"), pending comments are preserved instead of destroyed, and the new catalog key is correctly translated across all 13 locale files. Notably, the new code passes a translated verb where the older composer siblings pass English literals — it improves on the pattern rather than degrading it. No hedging copy, no silent-loss path remains, no divergence from product conventions.

UX-Verdict: PASS

Closes a silent comment-loss path with the composer's exact offline affordance — disabled state, translated tooltip, and preserved drafts all match established patterns.

[UX-REVIEWED] 787ae4d

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of 787ae4dd4da74dec6c8bbca5d738d03bec912241 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. Every entry point passing onSubmitComments originates at ChatPage and both render sites are plumbed; the offline-gating convention (offlineProps + caller-owned disabled + handler bail) is the documented existing mechanism, used, not duplicated; CommentList was already exported at base; en.json is generated so 13 locale files is the complete hand-edited set; temp-screenshots/ is an established convention (791 files). Here is the review:

First-Principles-Verdict: PASS

A reported silent-data-loss defect, fixed with the repo's own documented offline-gating mechanism, with no riders and complete entry-point coverage.

What this change ships

Intent: stop a user's pending review comments from being silently destroyed when they submit while the gateway is down. This is a FIX.

1. File panel "Submit All" disables with the offline tooltip/aria while disconnected — justified (named data-loss defect)
2. Artifact panel "Submit" gets the identical offline disable — justified (same defect, second entry point)
3. An offline click no longer composes-and-clears the pending comments — justified (the defect's core)
4. An offline submit no longer switches the active session with nothing sent — justified, declared
5. Optional connected prop (default true) threaded through six panel components — justified plumbing; each link has ≥1 real consumer (ChatPage → SidePanel → panels)
6. New utils.offline.submit_comments phrase in 13 locale files — mandated by the i18n invariant (en.json is generated; the set is complete)
7. One screenshot under temp-screenshots/ — follows repo convention (791 existing files)

The change reuses offlineProps (website/src/utils/offline.ts) exactly as its contract prescribes — caller-composed disabled, spread-after ordering — rather than inventing a second spelling, and offline.ts's docstring explicitly blesses layered per-entry-point guards, so the shape is convention-derived. I grepped for unfixed siblings: addSourceCommentToChat only appends to the composer input (nothing cleared, nothing sent), and no other render site passes onSubmitComments. Zero siblings remain.

Watch

submitAllComments still clears comments before send() confirms anything; this fix removes the offline instance of clear-before-confirm, the only reported one. If send() ever gains another silent-refusal condition, the destruction path reopens — a level note, not a demand.

[FIRST-PRINCIPLES-REVIEWED] 787ae4d

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 787ae4dd4da74dec6c8bbca5d738d03bec912241 via the fork AI-review pipeline; updated in place on each push.

Review details

The discovery pass found no candidates, and my review of the diff confirms it: this is a defensive offline-gating change that threads a connected prop through the comment-submit chain, gating both the button affordances and the behavioral handlers, with connected = true defaults preserving prior behavior. The offlineProps helper and locale keys line up correctly. No reachable defect, removed guard, or blocking-rule violation surfaced under falsification.

No findings.

[OPUS-REVIEWED] 787ae4d

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 787ae4dd4da74dec6c8bbca5d738d03bec912241 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 787ae4d