PT-4204: Don't open a second Comments panel on comment insert

[tombogle]

Code Review Summary

Branch: fix/PT-4204-suppress-second-comments-tab

Base: origin/main

Date: 2026-07-16

Review model: Claude Sonnet 5

Files changed: 1

Overview

This change fixes PT-4204: in Simple mode, inserting a comment from the Scripture editor was
opening a second, editor-anchored "Comments: " panel and stealing focus, even though the
new comment was already correctly reflected in the existing fixed Column 3 "Comments" tab. The fix
gates the single openCommentListAndSelectThreadSafe call site in onCommentEditorSave
(extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx) behind the
component's existing isPowerMode flag, so Power mode's behavior (open/focus the editor-anchored
panel and select the new thread) is unchanged, while Simple mode no longer opens the redundant
panel. The change is minimal (one file, ~8 lines) and includes an inline comment documenting why
the Simple-mode case is intentionally left unhandled here, satisfying the project's
cross-view-sync-hidden-views documentation rule. All four analysis passes (API/correctness,
style/patterns, coverage/compliance, UX) came back clean with no Critical or Important findings.

API Changes

None. The only changed file (platform-scripture-editor.web-view.tsx) is a WebView implementation
file, not a public API surface — no exports were added, modified, or removed in
lib/platform-bible-react/, lib/platform-bible-utils/, papi.d.ts, or any extension's
src/types/*.d.ts.

Findings

Critical — Must address before merge

None.

Important — Should address before merge

None.

Minor — Consider

• After this change, Power mode auto-selects/highlights the new comment thread in the comment
  panel, while Simple mode's comment now appears silently in the already-visible Column 3 tab with
  no equivalent highlight/selection feedback. (author's explanation: UX has been alerted and this
  will likely need a second pass once they decide on the desired Simple-mode feedback behavior; the
  priority for this PR was landing a fix for the obvious duplicate-panel bug.)
• The new isPowerMode branch has no automated test coverage — consistent with this
  component's existing convention (no test harness exists for platform-scripture-editor.web-view.tsx
  at all) but still a gap for the specific PT-4204 behavior. (author's explanation: not ideal, but
  acceptable for now. The attempted e2e test (see below) is noted in the PT-4204 Jira issue as a
  follow-up; plan is to revisit once the e2e environment is more stable/testable, likely alongside
  the UX follow-up.)

[Author response: Both minor findings were discussed and dismissed with context — no code changes
requested. Author has UX and Jira follow-ups already in motion for both.]

Template Propagation

Shared Regions Modified

None.

Extension Config Changes

None.

Positive Observations

• The isPowerMode guard precisely matches the PT-4204 fix's intent: it defaults to false in
  Simple mode, so gating openCommentListAndSelectThreadSafe behind it suppresses exactly the
  duplicate-panel behavior without touching Power mode.
• The useCallback dependency array was correctly updated to include the newly-referenced
  isPowerMode, avoiding a stale-closure bug.
• The inline comment at the guard clearly documents both the "what" and the "why" (Column 3's fixed
  Comments tab already reflects new comments via its own PDP subscription), satisfying the
  cross-view-sync-hidden-views rule's requirement for a durable, diff-visible justification rather
  than relying on the PR description alone.
• The change reuses the existing memoized isPowerMode value (already used consistently elsewhere
  in the file) rather than re-deriving mode state — good adherence to the file's established
  pattern.
• The change is minimal and surgical — 3 lines changed, 8 added, no unrelated modifications, no new
  UI strings, no localization/Storybook/build-config surface touched.
• Lint and formatting are clean on the changed file.
• The architectural claim in the comment (that openCommentList opens a distinct editor-anchored
  web view type from the fixed Column 3 panel) was independently verified against
  extensions/src/legacy-comment-manager/src/main.ts and confirmed accurate.

Interview Notes

• Stated purpose: Suppress the duplicate Comments panel that opens (and steals focus) when a
  comment is inserted from the editor in Simple mode. Explicitly scoped as Simple-mode-only per the
  bug report; no Power-mode behavior change intended or made.
• No Critical/Important findings — all four analysis passes returned clean.
• Minor finding 1 (Simple/Power feedback parity): dismissed. Author has already alerted UX to
  the feedback-parity gap; expects a second pass once UX decides what Simple-mode feedback should
  look like. This PR intentionally scopes to fixing the reported duplicate-panel bug only.
• Minor finding 2 (no automated test coverage): dismissed, acknowledged as not ideal. An e2e
  test was attempted (see below) but not committed; the gap is tracked in the PT-4204 Jira issue as
  a follow-up, to be revisited once the e2e environment is more stable — likely alongside the UX
  follow-up work.
• E2e test attempt: A regression test was written per the design doc's test plan
  (docs/superpowers/specs/2026-07-15-pt4204-suppress-comments-tab-on-insert-design.md) and run as
  part of the full comments-tab.spec.ts file (per project convention — this suite must run as a
  full file, not via --grep, since tests share a worker-scoped Electron instance). The run hit
  pre-existing, previously-documented environment flakiness (a cascading "Updating project view"
  overlay/PDPF-timeout failure starting partway through the file) unrelated to this fix — only 2 of
  14 test invocations passed across a 45-minute run, and the new PT-4204 test never got a stable app
  state to execute against. Per the design doc's own fallback plan, the test was not committed
  rather than landing something flaky/unverified.
• Manual verification: Both scenarios were manually verified by the author on the built branch:
  Simple mode — inserting a comment no longer opens a second Comments tab and the comment appears in
  the existing Column 3 tab; Power mode — the editor-anchored panel still opens/focuses and selects
  the new thread, unchanged.
• No additional context was volunteered beyond the above at wrap-up.

Suggested Review Focus

• Confirm the Simple-mode comment-insert feedback gap (no highlight/selection, vs. Power mode's
  auto-select) is acceptable to ship now, pending separate UX guidance — see Minor finding 1.
• No automated test coverage lands with this PR — confirm the manual verification described
  above (Simple: no second tab; Power: unchanged) is sufficient for merge, given the tracked Jira
  follow-up for e2e coverage.

AI-assisted — session

---

This change is 

[katherinejensen00]

Thanks for squashing this bug, Tom! Claude found four suggestions (literally weaker/less important than a minor or a warning) that you should feel free to ignore.

@katherinejensen00 reviewed 1 file and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on tombogle).

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1601 at r1 (raw file):

          );

          // Power mode only: Open the comment list and select the new thread. Simple mode's

NIT Claude Suggestion

After this fix, in Simple mode, creating a comment no longer calls openCommentListAndSelectThreadSafe — which means the selectThread(newThreadId) that normally highlights and scrolls to the new thread in the panel is also skipped. In Power mode the new thread is highlighted and scrolled into view immediately after creation; in Simple mode the comment appears in the Column 3 tab silently.

The comment says "Column 3's fixed Comments tab already reflects the new comment via its own PDP subscription." Does that subscription also auto-select/scroll-to the newest thread? If yes, there's no UX gap — the feedback just arrives differently. If no, Simple mode currently gives the user no visual confirmation of where their new comment landed.

Worth a quick verification before merge. If the PDP subscription doesn't auto-select, a follow-up to route a thread-select signal to the existing Column 3 controller (without opening a second panel) would close the gap cleanly.

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1606 at r1 (raw file):

          // second "Comments" tab and steal focus (PT-4204).
          if (isPowerMode) {
            await openCommentListAndSelectThreadSafe(papi, webViewId, newThreadId);

NIT Claude (Suggestion)
The createCommentAnnotationClickHandler at line 1240 calls openCommentListAndSelectThreadSafe without an isPowerMode guard — so in Simple mode, clicking an existing comment annotation in the editor still opens the editor-anchored Comments panel, even though this PR deliberately avoids that on the insert path.

Is that intentional? The two scenarios are meaningfully different: the create path is an implicit auto-open (the user didn't ask for it, so suppressing it is correct), while the click path is an explicit user action (the user is asking to navigate to a thread, so opening the panel may be acceptable). If the distinction is deliberate, a short comment on the click handler — analogous to the one you added here — would make it clear to the next reader that the asymmetry is intentional. If it's an oversight, the click handler may also need the guard.

Either way, this is not a defect introduced by this PR — it's a pre-existing call site — and the PT-4204 bug report is scoped to the insert path. Confirming the intent is all that's needed.

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1606 at r1 (raw file):

          // second "Comments" tab and steal focus (PT-4204).
          if (isPowerMode) {
            await openCommentListAndSelectThreadSafe(papi, webViewId, newThreadId);

NIT Claude Suggestion

No automated regression coverage lands with this fix. The mode-specific branch (isPowerMode → open panel vs. don't) could be silently broken by a future refactor — for example, if the guard is dropped or the two call sites (create path at 1605, click path at 1240) drift out of sync.

The PR description explains the e2e attempt hit pre-existing environment flakiness and a Jira follow-up is planned, which is a reasonable position given that no unit test harness exists for this WebView. One lower-cost alternative: a tiny Vitest unit test on a shouldOpenCommentListOnInsert(isPowerMode: boolean): boolean predicate (true in power mode, false in simple) would be a deterministic Revert Test that pins the intent for both call sites without touching the flaky e2e environment.

Not a blocker — the manual verification you described is solid and the gap is tracked. Just flagging the extraction option in case it's worthwhile before the e2e infrastructure stabilizes.

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1607 at r1 (raw file):

          if (isPowerMode) {
            await openCommentListAndSelectThreadSafe(papi, webViewId, newThreadId);
          }

NIT Claude Suggestion

Before this PR, openCommentListAndSelectThreadSafe was called unconditionally, so it always ran regardless of whether platform.interfaceMode loaded successfully. After this PR, it is gated on isPowerMode — which returns false when isPlatformError(interfaceModePossiblyError) is true (line 388).

This means a Power-mode user who hits a platform.interfaceMode setting read failure will now silently get Simple-mode behavior: their newly inserted comment's panel won't open or select the new thread. The fallback to false is correct for users who are actually in Simple mode, but it's a silent degradation for Power users when the setting itself is the thing that failed.

A logger.warn at line 388 when isPlatformError is true (e.g. "platform.interfaceMode failed to load — falling back to simple mode behavior for comment insert") would make this diagnosable in the field at negligible cost.

[tombogle]

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1601 at r1 (raw file):

Previously, katherinejensen00 wrote…

NIT Claude Suggestion

After this fix, in Simple mode, creating a comment no longer calls openCommentListAndSelectThreadSafe — which means the selectThread(newThreadId) that normally highlights and scrolls to the new thread in the panel is also skipped. In Power mode the new thread is highlighted and scrolled into view immediately after creation; in Simple mode the comment appears in the Column 3 tab silently.

The comment says "Column 3's fixed Comments tab already reflects the new comment via its own PDP subscription." Does that subscription also auto-select/scroll-to the newest thread? If yes, there's no UX gap — the feedback just arrives differently. If no, Simple mode currently gives the user no visual confirmation of where their new comment landed.

Worth a quick verification before merge. If the PDP subscription doesn't auto-select, a follow-up to route a thread-select signal to the existing Column 3 controller (without opening a second panel) would close the gap cleanly.

I had noted this potential deficiency in a comment on the issue and tagged UX for input. Sebastian (at least) thinks it should select the newly-inserted comment, which is easy enough to do. (There is the edge case where an in-process edit might already be happening in the Comments tab for some other comment, but that is already called out as an issue to be dealt with in the more general case of synchronized scrolling.)

[tombogle]

@tombogle made 2 comments and resolved 1 discussion.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on katherinejensen00).

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1606 at r1 (raw file):

Previously, katherinejensen00 wrote…

NIT Claude (Suggestion)
The createCommentAnnotationClickHandler at line 1240 calls openCommentListAndSelectThreadSafe without an isPowerMode guard — so in Simple mode, clicking an existing comment annotation in the editor still opens the editor-anchored Comments panel, even though this PR deliberately avoids that on the insert path.

Is that intentional? The two scenarios are meaningfully different: the create path is an implicit auto-open (the user didn't ask for it, so suppressing it is correct), while the click path is an explicit user action (the user is asking to navigate to a thread, so opening the panel may be acceptable). If the distinction is deliberate, a short comment on the click handler — analogous to the one you added here — would make it clear to the next reader that the asymmetry is intentional. If it's an oversight, the click handler may also need the guard.

Either way, this is not a defect introduced by this PR — it's a pre-existing call site — and the PT-4204 bug report is scoped to the insert path. Confirming the intent is all that's needed.

I think this should be fixed as part of this fix as well. Since annotations currently don't display (a different bug), this is not currently observable/testable, but it really is just another way to get into this same undesirable situation.

---

extensions/src/platform-scripture-editor/src/platform-scripture-editor.web-view.tsx line 1607 at r1 (raw file):

Previously, katherinejensen00 wrote…

NIT Claude Suggestion

Before this PR, openCommentListAndSelectThreadSafe was called unconditionally, so it always ran regardless of whether platform.interfaceMode loaded successfully. After this PR, it is gated on isPowerMode — which returns false when isPlatformError(interfaceModePossiblyError) is true (line 388).

This means a Power-mode user who hits a platform.interfaceMode setting read failure will now silently get Simple-mode behavior: their newly inserted comment's panel won't open or select the new thread. The fallback to false is correct for users who are actually in Simple mode, but it's a silent degradation for Power users when the setting itself is the thing that failed.

A logger.warn at line 388 when isPlatformError is true (e.g. "platform.interfaceMode failed to load — falling back to simple mode behavior for comment insert") would make this diagnosable in the field at negligible cost.

Probably worth considering.