fix(chat): remove follow-up options only from generated suffix

[aniruddhaadak80]

Problem / Motivation

ChatPage's follow-up option toggle used substring search when removing an option. A matching fragment inside the user's draft could be deleted instead of the text generated by the toggle. A simple lastIndexOf still corrupts an earlier , Go if the user manually removes the generated tail before unselecting the chip.

Why it matters

Unselecting a suggested answer must never rewrite unrelated draft text. Draft corruption can silently change what the user sends, and the bug becomes especially easy to trigger with short options such as Go.

What changed

• Treat the currently picked options as one ordered, generated suffix.
• Remove an option only when that complete suffix is still present at the end of the composer.
• Rebuild the suffix from the remaining picked options, preserving the user's draft and multi-select order.
• If the user edited the generated structure, only unmark the chip and leave their text untouched.
• Drop the branch's carried backend liveness commit because main already contains the more complete fix: keep watching an app backend's health after startup #5929 implementation.

Tests

• Added a deterministic regression for Please, Google, Go unselecting back to Please, Google.
• Added the edited-tail negative case: after the user removes the appended option, unselecting leaves Discuss, Go home untouched.
• Focused follow-up toggle suite: 12 passed.
• Related ChatPage matrix: 374 passed.
• Typecheck, lint (0 errors), and production build passed.
• The pre-fix lastIndexOf implementation was mutation-checked and deterministically failed the edited-tail case.

No retry, sleep, timeout increase, or flaky-test suppression was used.

Manual verification

N/A — the interaction state machine is covered directly with the real ChatPage component and fake timers.

Screenshots / video

Why no screenshot: This fixes composer text mutation without changing rendered layout or styling; the before/after behavior is pinned by the interaction tests above.

Related Issues

Fixes #5897

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

Blocking: the description presents this as the two indexOf-to-lastIndexOf edits in ChatPage, but the current main-targeting diff also contains the app-backend liveness implementation and its tests from the preceding commit (src/kiro_crew/apps/backend.py plus two backend test files). Please rebase/split that unrelated change out, or explicitly describe and justify the combined scope and update the evidence.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

Full-diff overlap audit (current head e9fde575061d3b1cb9ffff22fde5c2e8ec127ac8): this head literally contains the complete #6079 commit (d12e03b0d8e670f736f5c2e50940c31831faa014) plus one independent change in ChatPage.tsx. Merged #5929 (ea4116f6b6323d2ee3e6a8e4d05e91b907f25172) already covers #6079's process/HTTP watcher, demotion and supervisor behavior with a broader generation-fenced/recoverable implementation.

The residual is still valid: changing the follow-up session lookup from indexOf to lastIndexOf, with its focused test, is not in #5929 or current main. Please rebase onto current main, drop the carried #6079 commit, and leave only that ChatPage hunk/test. #5727 should independently be reduced to its safe no-proxy/no-redirect health-probe residual.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Why it matters

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

Resolved on 5a5088b: the unrelated app-backend commit and all backend diffs were dropped; the PR is now one ChatPage suffix commit with matching tests and description.

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

No findings.
[GPT-REVIEWED] 5a5088b

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

Design-Verdict: PASS

Whole-suffix matching is the right shape: fail-safe toward leaving the draft alone, proportionate, and pinned by deterministic regression tests.

Watch

• The "user edited → text untouched" guarantee is still heuristic, not total: a draft that coincidentally ends with the exact delimited suffix (draft Yes, Go, pick Go, hand-delete the appended , Go, then unselect) still truncates to Yes, because endsWith(', ' + pickedSuffix) cannot tell the user's comma from the generated one. This is a far narrower corner than the bug fixed, and closing it fully needs a real append boundary (tracked offset or a structured draft+chips composer) — disproportionate here, but worth knowing the residual exists if ChatPage follow-up unselect splices matching draft substring (indexOf vs lastIndexOf) #5897-class reports recur.

[DESIGN-REVIEWED] 5a5088b

[github-actions]

UX Review (Fable 5, fork) — ✅ PASS

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

The patch is a targeted behavioral fix: unselecting a follow-up chip previously spliced the first ", Go" substring match anywhere in the draft — corrupting user-typed text like "Please, Google" — and now removes only the intact generated suffix at the composer's end, falling back to unmark-only (chip dims, text untouched) when the user has edited it. I've verified the trade-offs: the worst pre-fix outcome (silent corruption of what the user sends) is replaced by a conservative no-op with visible chip feedback, the conservative fallback was already the documented design for edited drafts, multi-select order is preserved, and no strings, layout, or visual surface changes. The narrowed removal (editing anywhere in the tail leaves all option text in place) is rare, explicitly chosen, and strictly safer than the corruption it prevents.

UX-Verdict: PASS

Unselecting a chip can no longer silently rewrite the user's draft; the conservative fallback keeps visible chip feedback and never touches edited text.

[UX-REVIEWED] 5a5088b

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 5a5088bdcb2f25131582fea8e95c0efb03e86f2b 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.

The review is complete. The fix in ChatPage is sound and cause-level, but the identical toggle in ChatPane.tsx still carries the lastIndexOf approach that the PR's own description declares buggy — a counted unfixed sibling.

First-Principles-Verdict: CONCERNS

The ChatPage fix is cause-level, but the identical toggle in ChatPane still uses the lastIndexOf splice the description itself calls corrupting.

What this change ships

Intent: unselecting a suggested-answer chip must never rewrite the user's draft text — a FIX.

1. Unselect removes text only when the full generated suffix is intact at the composer's end — justified, cause-level
2. Unselecting a middle option rebuilds the tail from remaining picks, order kept — declared, part of fix
3. After any hand-edit of the generated tail, unselect only un-highlights and leaves text — declared, stricter form of the existing fallback
4. Two regression tests (substring-in-draft, edited-tail) — justified

Watch

• One unfixed sibling with the same root cause. Grep ', ' + o in website/src: 2 production sites — this one and ChatPane.tsx:828-843, an identical toggle that still uses prev.lastIndexOf(', ' + o). The description states "A simple lastIndexOf still corrupts an earlier , Go if the user manually removes the generated tail before unselecting the chip" — so by the author's own analysis, ChatPane remains bug-for-bug at that level, and its regression test (ChatPane.followUpOptions.test.tsx:195) pins the weaker behavior. The same ~10-line suffix rewrite applies there; this lands as a point patch until it does, and the two copies now diverge in behavior.

[FIRST-PRINCIPLES-REVIEWED] 5a5088b

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 5a5088b

[bolichen97]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: removes follow-up options only from the generated ordered suffix (ChatPage), fixing a substring-match that corrupted an earlier ', Go' in the user's own draft — pure input-string edit, no security surface. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.

[bolichen97]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: replaces substring removal of a follow-up option with removal of the complete generated suffix, so an earlier match inside the user's own draft is no longer spliced; covered by two regression tests. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.

[iamwhatever]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: the follow-up-options stripper ran over the whole message body, so an options-shaped line the user themselves wrote was removed — root cause is the unscoped match in ChatPage.tsx; fix confines removal to the generated suffix, with a toggle regression test. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #7616 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7616: KEEP. 6092 is the source of the ported logic, not coverage of it. The grid pane is still defective on current main, so this PR closes the divergence 6092 left behind. Files: website/src/pages/ChatPage.tsx.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.