FE-656: Side-chat V1.2 — vertical slice (annotate)#88
Merged
Conversation
FE-656 Side chat
Side chat — graph-launched chat with patch-list stagingTL;DRProblem / Motivation
Acceptance criteria
ConceptPopover-to-panel chat anchored to items in the structured spec view. Two entry modes (per-row The side-chat is the unified user-driven mutation surface. It subsumes three previously-separate horizon items: D128 graph-launched refinement, trigger-popover composer, revisit / edit mode + cascade preview. Phasing
Acceptance-criteria mapping
Design doc & PR
|
7 tasks
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
kostandinang
changed the title
kostandinang
changed the title
PR SummaryMedium Risk Overview Introduces a new client Reviewed by Cursor Bugbot for commit 4e3ac78. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: This PR delivers Side-chat V1.2 “Annotate” as a vertical slice, proving a durable mutation seam from the popover UI through a patch list to persisted database writes. Changes:
Technical Notes: Patch-list state is derived via a pure fold over an append-only 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
kostandinang
added a commit
that referenced
this pull request
May 4, 2026
augmentcode[bot] flagged two medium-severity gaps where V2 patch kinds could silently misbehave when added later. Both are pre-emptive fixes that change no V1.2 behavior. patch-list-host.tsx — apply() exhaustive switch - Old: `if (patch.kind === 'annotate')` silently skipped non-annotate kinds, but APPLY_SUCCESS still removed all staged ids from the list, so future V2 patches would be marked applied without ever running their applier. - New: switch with a `default` branch that asserts `never` on patch.kind, so adding a new Patch kind to the closed union without a case here is a typecheck failure. side-chat-host.tsx — auto-apply kind guard - Old: `useEffect` triggered on any staged-count growth, would auto-apply V2 edit / drill-down / propose-edge patches that need explicit Apply per D131. - New: only fires when every staged patch has kind === 'annotate'. 793/793 tests pass. No new tests added — V1.2 only has annotate, so the new branches aren't reachable in the current test surface; they guard future regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kostandinang
added a commit
that referenced
this pull request
May 4, 2026
Three issues, all in the auto-apply / annotation-fetch effects: side-chat-host.tsx — auto-apply uses a "triggered IDs" set instead of count-based comparison - Bug: with `current > previous`, a patch staged during an in-flight apply would be missed. Sequence: stage A → apply runs → user stages B mid-flight → APPLY_SUCCESS removes A from staged → next render has current=1, previous=2 → effect skips B. B sits stuck even though the server is healthy. - Fix: a Set of patch IDs we've already triggered apply for. Effect fires whenever there's a staged patch ID we haven't seen yet AND all staged are auto-applyable AND we're not already applying. Old IDs are pruned when they leave staged. Failure case still avoids loops: after APPLY_FAILURE the patch stays staged and stays in the set, so the effect doesn't refire. side-chat-host.tsx — annotation refetch deps - Bug: `useEffect(..., [activeSideChat, ...])` re-fired on every annotateMode toggle since toggling produces a new object reference. - Fix: depend on the primitive `activeItemId` and `activeItemKind` instead of the whole `activeSideChat` object. Refetch now only fires on actual pin changes + canUndo/isApplying transitions. patch-list-host.tsx — useStagedPatches stable filter deps - Bug: useMemo depended on `filter?.anchor` (an object). Inline filter objects from callers always have a new reference, so the memo never prevented recomputation. - Fix: depend on the primitive `filter?.anchor?.kind` and `filter?.anchor?.itemId` instead. 793/793 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kostandinang
force-pushed
the
ka/fe-656-side-chat-v1-2
branch
from
f49fa7d to
2a18e2f
Compare
6 tasks
Contributor
Author
Cursor Bugbot review status (round 2)All five comments are addressed:
|
kostandinang
added a commit
that referenced
this pull request
May 5, 2026
- Drop debug "(retry?)" text from staged-annotations header in side-chat popover; the retry affordance is the dedicated Retry button below. - Guard PatchListProvider.apply() with a synchronous in-flight ref so rapid re-entries (e.g. double-click on Retry) cannot race past the closure-captured isApplying check and produce duplicate server writes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kostandinang
added a commit
that referenced
this pull request
May 5, 2026
Annotation refetch effect used `patchListState.canUndo` as its trigger, but `canUndo` stays `true` across consecutive applies — so the second save never refetched, and undoing a non-head batch never refreshed. Use `lastBatchId` instead, which mutates per apply and on undo of the head batch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kostandinang
added a commit
that referenced
this pull request
May 5, 2026
…ring Resolve conflicts in memory/PLAN.md and memory/SPEC.md where main and the side-chat branch independently allocated overlapping spec IDs (Requirement 34; A71-A73; D130-D131). Keep the side-chat branch's numbering since the stacked PR #88 and docs/design/SIDE_CHAT.md already commit to it; renumber main's provider-setup / gitignore items to follow: - Requirements 34..37 (provider/gitignore) → 35..38 - A71..A73 (provider/gitignore) → A74..A76 - D130..D133 (provider/gitignore) → D132..D135 - Update I106/I107 cross-references; update PLAN.md horizon traceability Drop the "Revisit / edit mode + cascade preview" horizon item — it's superseded by Side-chat V3 per D130. Verified: npm run verify (726 tests pass, build clean).
kostandinang
added a commit
that referenced
this pull request
May 5, 2026
The previous merge resolution put provider-setup decisions at D132-D135, but PR #88 (ka/fe-656-side-chat-v1-2, stacked above this branch) already uses D132 for the PatchListProvider module and D133 for the new annotation entity with side-chat semantics. Shift provider items to D134-D137 so PR #88's existing numbering can ride through the rebase without a new conflict. - D134..D137 = first-run setup / provider seam / UI credentials / gitignore - A74-A76 dependency refs updated - I106/I107 traceability refs updated - PLAN.md horizon traceability refs updated Verified: npm run verify (726 tests pass, build clean).
kostandinang
force-pushed
the
ka/fe-656-side-chat-v1-2
branch
from
8787454 to
be46b96
Compare
7 tasks
lunelson
previously approved these changes
May 6, 2026
…ity, queue cards Kicks off V1.2 (Side-chat Annotate vertical slice) on a stacked branch over V1.1's PR #81. The /ln-design synthesis settled the patch-list module shape and the annotation entity model; /ln-scope queued the three vertical-slice cards (server seam, client module, end-to-end wiring) in CARDS.md. memory/SPEC.md - Add D132: patch-list module is React-context-native publicly, event-log- shaped internally. Provider + 3 hooks; closed discriminated union for patch kinds; useReducer over PatchEvent log; appliers prop forces V2 kinds to typecheck-fail at mount until supplied. Internal events shaped to match A71's eventual server-side primitive — migration is reducer swap, not API rewrite. - Add D133: annotation is a new durable entity with its own table. Item- anchored in V1; `selection_start`/`selection_end` columns are part of the schema from day one but stay NULL until V2/V3 lights up span anchors. Supersedes the optimistic "comment-store extension" framing — the spike showed there is no prior comment store; per-turn `itemComments` on review responses is a separate seam and stays unchanged. memory/PLAN.md - Update Active 1 to reflect V1.2 in progress on `ka/fe-656-side-chat-v1-2`, reference the CARDS.md vertical-slice queue, and correct the "comment- store extension" framing per D133. - Add D132 / D133 to traceability; record both branches under Linear FE-656. memory/CARDS.md (new) - Card A: annotation server seam — drizzle migration, table with FK + cascade, POST/GET/DELETE endpoints, server-only tests. - Card B: PatchListProvider client module — sibling to SideChatHost, hooks for mutations + reactive reads + filtered selectors, internal reducer + pure fold tests. - Card C: end-to-end annotate wiring — annotate composer in side-chat header, in-panel inline patch list, in-panel Apply hits the new endpoint, Undo round-trips. Top-bar canonical UI, floating selection menu, and multi-pin re-scope after Card C lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the durable `annotation` entity per D133 — the foundation for the
side-chat's user-driven mutation surface (D130/D131). Server-pure CRUD
seam; no client wiring yet (Cards B and C extend it).
Schema (drizzle/0013_annotation.sql + src/server/schema.ts)
- New `annotation` table: id, specification_id (FK), knowledge_item_id
(FK ON DELETE CASCADE), summary, body, selection_start (NULL), selection_end
(NULL), created_at default datetime('now').
- selection_start / selection_end columns are NULL placeholders for V2/V3
span anchoring per D133; V1 leaves them NULL.
DB helpers (src/server/db.ts)
- createAnnotation, getAnnotationsForSpecification, getAnnotation,
deleteAnnotation. Annotation type exported via InferSelectModel.
Endpoints (src/server/annotation-route.ts + src/server/app.ts)
- POST /api/specifications/:id/annotations — zod-validated; resolves
(itemKind, itemId) against the spec's project-wide entities; 201 with
the new row; 400 on bad payload; 404 on spec/item miss.
- GET /api/specifications/:id/annotations — chronological list scoped
to the spec; empty array when none.
- DELETE /api/annotations/:annotationId — idempotent; 204 even when the
row is already gone (matches the V1 patch-list optimistic undo shape).
- App-level adds a registerDelete helper for symmetry with registerGet/Post.
Tests
- src/server/db.test.ts +5 tests: round-trip, span-anchor columns,
per-spec scoping, delete + cascade-on-knowledge_item-delete.
- src/server/annotation-route.test.ts (new) +20 tests: POST/GET/DELETE
happy paths, validation, 404 paths, cross-spec isolation, kind matrix,
cascade behavior end-to-end.
- 712/712 tests pass (V1.1 had 687).
CARDS.md
- Mark Card A done. Card B (PatchListProvider) is next.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the patch-list module per D132: React-context-native public API
(provider + 3 hooks), event-log internal primitive (useReducer over
PatchEvent), closed discriminated union for patch kinds (V1.2: annotate
only). No route mount yet — that ships with Card C alongside the real
annotate applier.
Functional core (patch-list-reducer.ts)
- PatchEvent discriminated union: PatchStaged / PatchDiscarded /
PatchSummaryEdited / BatchApplied / BatchUndone.
- patchListReducer: pure state transitions over the event log + the
non-serializable sidecar (isApplying flag, pendingUndos map for
undo handles). Sidecar exists because undo handles are closures, not
data; the reducer comment explains the A71 forward-compat seam.
- deriveState: pure fold over events → { staged, count, canUndo,
isApplying, lastBatchId }. After undo, the undone batch's patches
re-stage in original order so the user can re-apply or revise.
- getPendingUndoHandle: finds the most-recent un-undone batch's handle
for the React layer to invoke during undo.
Public surface (patch-list-host.tsx)
- <PatchListProvider specificationId appliers idFactory? now?>: mounts
the reducer, wires deriveState through useMemo, exposes the
context value. idFactory/now are test-only seams; production uses
crypto.randomUUID() and Date.now().
- usePatchList(): returns null outside the provider; returns
{ stage, discard, editSummary, apply, undo } inside.
- usePatchListState(): returns { staged, count, canUndo, isApplying }
reactively; safe-defaults outside the provider so consumers can render
unconditionally.
- useStagedPatches({ anchor?, kind? }): filtered selector for in-panel
inline surfacing (Card C uses { anchor } to show patches on the
pinned item).
- PatchAppliers is a typed record keyed by patch.kind. V2 patch kinds
add keys; provider-mount sites typecheck-fail until the new applier
is supplied. This is the closed-union pressure D132 promised.
Apply / undo orchestration
- apply: snapshots staged, dispatches APPLY_START, fans out to
appliers[patch.kind] sequentially, builds a single batched undoAll
closure, dispatches APPLY_SUCCESS or APPLY_FAILURE. No-op when staged
is empty or already applying.
- undo: looks up the most-recent un-undone batch via
getPendingUndoHandle, awaits the closure, dispatches UNDO_SUCCESS.
Best-effort per D132 — failures are swallowed (toast surfacing
deferred to a later card).
Tests
- patch-list-reducer.test.ts (20 tests): every action type, every
derived-state transition, full sequence round-trip.
- patch-list-host.test.tsx (14 tests): provider mount, hook contracts,
stage/discard/editSummary, apply happy path + empty + failure, undo
reverse-order + canUndo flip + no-op, useStagedPatches anchor/kind
filters, returning full list when filter omitted.
- 746/746 tests pass (Card A landed at 712 → +34 for Card B).
Note: CARDS.md updated — route.tsx mount moved from Card B's boundary
list to Card C, since mounting without the real annotate applier
(annotation-api.ts, Card C's territory) adds no test value over the
in-memory provider tests already in place.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the V1.2 vertical slice. Side-chat panel can now produce durable
annotations via the patch list: annotate composer in the popover header
stages a patch through usePatchList(); the in-panel inline list surfaces
staged patches; Apply fans out to POST /api/specifications/:id/annotations;
Undo deletes the created row. The seam from chat -> patch -> durable spec
mutation is end-to-end working.
annotation-api.ts (new)
- createAnnotationRequest / deleteAnnotationRequest fetch wrappers,
following the existing side-chat-stream pattern (injectable fetch for
tests).
- makeAnnotateApplier(specificationId) produces the ApplyPatchFn the
PatchListProvider consumes; the applier closes over the created
annotation's id so undo deletes the right row.
side-chat-popover.tsx
- Header gains an Annotate button (rendered only when onAnnotateRequest
is provided; disabled while the chat stream is in flight).
- annotateMode replaces the chat input with a composer (summary + body +
Cancel + Stage). Esc cancels the composer instead of dismissing the
popover when in annotateMode.
- Inline patch list section renders below the messages when there are
staged patches OR canUndo=true (the latter so Undo stays reachable
after Apply clears staged; transitional UX gap that V1.2-D's
canonical top-bar will close per D131).
- Per-row Discard, batch Apply / Undo buttons. isApplying disables both.
side-chat-host.tsx
- ActiveSideChat gains annotateMode flag; new requestAnnotate /
cancelAnnotate / submitAnnotate callbacks. submitAnnotate calls
usePatchList().stage(...) with the pinned item as anchor and exits
annotate mode.
- usePatchListState() / useStagedPatches({ anchor, kind: 'annotate' })
feed the popover's inline list filtered to the currently pinned item;
switching pins isolates each item's staged patches.
- Annotate button is conditionally surfaced: present only when
usePatchList() returns non-null (i.e., a PatchListProvider is in
scope). Side-chat degrades gracefully without one.
route.tsx
- Wraps SideChatHost with PatchListProvider per D132 mount order.
The annotate applier is memoized against specification.id so the
closure is stable across renders.
Tests (+24)
- side-chat-popover.test.tsx +18: annotate button rendering / disable
semantics / click; annotateMode composer; Stage button enablement;
trimmed-summary submission; Cancel + Esc cancel; inline list /
rows / Discard; Apply + Apply-disabled-while-applying; Undo gated
on canUndo.
- side-chat-host.test.tsx (new) +8: open -> annotate -> composer;
stage -> inline list; Apply success + Undo round-trip; Apply failure
preserves staged; Discard removes; pin-isolated filtering; graceful
no-provider fallback.
- 770/770 tests pass (Card B landed at 746 -> +24 for Card C).
Reconciliation
- memory/CARDS.md retired vertical-slice queue exhausted.
- memory/PLAN.md updated vertical slice landed, V1.2-D/E/F still owed
with fresh /ln-scope. V1 frontier item still Active.
- D131 / D132 / D133 unchanged Card C implements them; no SPEC drift.
Known transitional gap: Undo lives only in the in-panel inline list
until V1.2-D ships the top-bar canonical surface. Per D131 the in-panel
list is "convenience UI, not source of truth" once V1.2-D lands, the
canonical Undo moves to the persistent top-bar.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the explicit "click Stage, click Apply" UX for user-driven annotations: typing the note and submitting the composer now persists it directly. The patch list still records the patch, surfaces it transiently in the inline list, and provides Undo on the resulting batch. System-driven future annotations (V4 architect-loop) and all V2 patch kinds (edit / drill-down / propose-edge) keep the explicit Apply step — review-before-commit matters there because the producer isn't the user typing in the moment. memory/SPEC.md - Revise D131 to make the user-driven vs review-required distinction explicit, supersede the "uniform staging required" framing. src/client/components/side-chat-host.tsx - Add a useEffect that fires patchList.apply() when staged count grows. - Guard with prevStagedCountRef so a failed apply doesn't loop — count staying at 1 after APPLY_FAILURE doesn't satisfy current > previous. src/client/components/patch-list-reducer.ts - BatchUndone is terminal: undone patches don't re-stage. If they did, the auto-apply effect would immediately reapply them. V2 patch kinds that benefit from re-staging-on-undo (preserves an in-flight edit) will re-introduce it gated by kind. Test updates - patch-list-reducer.test.ts: undo expectations flip from "patches re-stage" to "staged stays empty, canUndo false". - patch-list-host.test.tsx: count after undo is 0, not 2. - side-chat-host.test.tsx: clicking Stage triggers auto-apply (no second click needed). Failure-path test asserts the in-panel Apply button is back as the explicit retry. The discard test and the anchor-filter test use a failing applier so staged patches dwell long enough to exercise those paths — the realistic dwell scenario for V1.2 is the failure case. - 770/770 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The inline patch list section was rendering as an empty gray bar after auto-apply succeeded — staged was empty (so the patch rows hid) but canUndo kept the section mounted, leaving just a thin Undo button alone. Adds an explicit "Annotation saved." confirmation alongside Undo so the section has meaning when there's nothing staged but the user can still undo the most recent batch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the staging language now that user-driven annotations auto-apply. Three distinct visible states replace the one overloaded "staging area" that flashed during success and lingered awkwardly afterward: - "Saving annotation..." status while isApplying is true - "✓ Annotation saved" confirmation with Undo when staged is empty and canUndo is true - Pending-annotations panel with Retry / Discard only when staged is non-empty AND not currently applying (i.e., a stuck/failed batch) Composer button: "Stage" → "Save". Failed-batch panel says "1 pending annotation (retry?)" instead of "1 staged annotation"; the retry action is labeled "Retry" instead of "Apply" since the user-driven auto-apply path doesn't expose explicit Apply anymore. The staging panel no longer renders during in-flight auto-apply, so the brief "1 staged annotation" + "Applying…" flash is gone. The saved-confirmation row sits in its own light surface (bg-wash/40) distinct from the staging-area styling (bg-wash/60), so it reads as confirmation rather than leftover staging. Test updates - popover: 6 tests reframed around saving / saved / stuck states; Stage→Save renames; staging panel only asserted in the stuck case. - host: button-name renames; assertions tracking "1 staged annotation" → "1 pending annotation" or "annotation saved"; failure-path retry button check. - 771/771 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the "save but don't show" gap. When the side-chat panel opens for
an item, the panel fetches annotations and renders the ones for that
item as a "Notes" section above the message log. After auto-apply
succeeds (canUndo flips), the section refetches so the just-saved note
appears without an optimistic update path.
Each note is a collapsible <details>/<summary> row: the summary text is
the click target, the body expands inline. When body equals summary (or
is empty), the row renders flat without a chevron — no point in
expanding to see the same text again.
annotation-api.ts
- Add listAnnotationsForSpecificationRequest(specId, options?). Mirrors
the existing fetch-wrapper pattern; injectable fetch for tests.
side-chat-popover.tsx
- New existingAnnotations prop: readonly { id, summary, body }[].
- Renders between the header and the message log when non-empty.
- Per-row: collapsible <details> with chevron when body differs from
summary; flat row otherwise.
side-chat-host.tsx
- useState<readonly CreatedAnnotation[]>([]) for fetched annotations.
- useEffect refetches on mount (when activeSideChat is set), on
specificationId change, and whenever canUndo or isApplying transitions
— so the list re-syncs after apply success and undo.
- Filters fetched annotations to the pinned item via knowledge_item_id
before passing to the popover.
Tests: 780/780 pass (771 existing + 9 from V1.1 polish merge; no Card
C2-specific tests added — fetch is straightforward and the popover
rendering is covered by the existing snapshot of UI structure).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wraps the existing notes list in a <details>, open by default, so the section header doubles as a click-target for collapsing the whole list. Each row is still independently expandable for body inspection. The border-b under the section is gone — it sat flush above the message log and read as a divider where none was needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
augmentcode[bot] flagged two medium-severity gaps where V2 patch kinds could silently misbehave when added later. Both are pre-emptive fixes that change no V1.2 behavior. patch-list-host.tsx — apply() exhaustive switch - Old: `if (patch.kind === 'annotate')` silently skipped non-annotate kinds, but APPLY_SUCCESS still removed all staged ids from the list, so future V2 patches would be marked applied without ever running their applier. - New: switch with a `default` branch that asserts `never` on patch.kind, so adding a new Patch kind to the closed union without a case here is a typecheck failure. side-chat-host.tsx — auto-apply kind guard - Old: `useEffect` triggered on any staged-count growth, would auto-apply V2 edit / drill-down / propose-edge patches that need explicit Apply per D131. - New: only fires when every staged patch has kind === 'annotate'. 793/793 tests pass. No new tests added — V1.2 only has annotate, so the new branches aren't reachable in the current test surface; they guard future regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three issues, all in the auto-apply / annotation-fetch effects: side-chat-host.tsx — auto-apply uses a "triggered IDs" set instead of count-based comparison - Bug: with `current > previous`, a patch staged during an in-flight apply would be missed. Sequence: stage A → apply runs → user stages B mid-flight → APPLY_SUCCESS removes A from staged → next render has current=1, previous=2 → effect skips B. B sits stuck even though the server is healthy. - Fix: a Set of patch IDs we've already triggered apply for. Effect fires whenever there's a staged patch ID we haven't seen yet AND all staged are auto-applyable AND we're not already applying. Old IDs are pruned when they leave staged. Failure case still avoids loops: after APPLY_FAILURE the patch stays staged and stays in the set, so the effect doesn't refire. side-chat-host.tsx — annotation refetch deps - Bug: `useEffect(..., [activeSideChat, ...])` re-fired on every annotateMode toggle since toggling produces a new object reference. - Fix: depend on the primitive `activeItemId` and `activeItemKind` instead of the whole `activeSideChat` object. Refetch now only fires on actual pin changes + canUndo/isApplying transitions. patch-list-host.tsx — useStagedPatches stable filter deps - Bug: useMemo depended on `filter?.anchor` (an object). Inline filter objects from callers always have a new reference, so the memo never prevented recomputation. - Fix: depend on the primitive `filter?.anchor?.kind` and `filter?.anchor?.itemId` instead. 793/793 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Reset annotate composer state on item switch (key by sessionId) and on exiting annotateMode, so drafts can't leak across pinned items. - Drop patchListState.isApplying from the annotations-fetch effect deps; canUndo alone covers post-save/undo refetch and avoids stale fetches during in-flight apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop debug "(retry?)" text from staged-annotations header in side-chat popover; the retry affordance is the dedicated Retry button below. - Guard PatchListProvider.apply() with a synchronous in-flight ref so rapid re-entries (e.g. double-click on Retry) cannot race past the closure-captured isApplying check and produce duplicate server writes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Annotation refetch effect used `patchListState.canUndo` as its trigger, but `canUndo` stays `true` across consecutive applies — so the second save never refetched, and undoing a non-head batch never refreshed. Use `lastBatchId` instead, which mutates per apply and on undo of the head batch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
lunelson
force-pushed
the
ka/fe-656-side-chat-v1-2
branch
from
7ddda47 to
f14faed
Compare
lunelson
previously approved these changes
May 7, 2026
Keep the provider API honest by deriving specification scope from appliers instead of accepting an ignored prop. Co-authored-by: Cursor <cursoragent@cursor.com>
github-actions
Bot
dismissed
lunelson’s stale review
See the output of git range-diff at https://github.com/hashintel/brunch/actions/runs/25484938618
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b3b9d9c. Configure here.
Prevent the side-chat saved confirmation from following a completed annotation batch when the user switches pinned items. Co-authored-by: Cursor <cursoragent@cursor.com>
lunelson
approved these changes
May 7, 2026
graphite-app Bot
pushed a commit
that referenced
this pull request
May 7, 2026
Stacked on PR #88. Lands **V1.2-G** — Figma §11 brand-surface polish — on top of the V1.2 vertical slice. https://www.loom.com/share/c87ef01e7e2a45429b48c3e51ea66fd7 ## Changes - **Panel layout** — 588px-wide popover with frosted-glass + kind-tinted halo (using shared `kindAccentHex` from `knowledge-card`) for both docked and floating modes. - **Docked / floating toggle** — Gmail-style float at bottom-right; docked mode pushes page content left and persists in `localStorage` (guarded against blocked / sandboxed storage). - **Composer** — ChatGPT-style circular Send inside the input, `+` and mic placeholders, Loader2 spin while streaming. - **Streaming reveal** — character-by-character typewriter for assistant messages. - **Action toolbar** — white-card Annotate / Edit / Cancel buttons with collapsible Notes overlay on the left. - **Top-bar icons** — borderless, theme-matched. - **Scrollbars** — thin themed `.scrollbar-thin` utility; outer `html/body` scroll suppressed so only inner page areas scroll. - **Dismiss behavior** — click-outside no longer closes the popover; the panel is non-modal, so the previous Tab focus trap was dropped to keep keyboard users free to tab back into the page (Escape still dismisses). ## Test plan - [ ] Open side-chat from a knowledge item → panel opens with kind-tinted halo matching the item's color. - [ ] Toggle dock ↔ float, reload — preference persists; no crash if localStorage is blocked. - [ ] Docked mode pushes content; only the inner page scrolls (no outer browser scrollbar). - [ ] Stream a response — typewriter reveals characters; Send shows spinning loader. - [ ] Tab from inside the panel exits to the page; Escape closes. - [ ] Annotate flow still works end-to-end (D131 auto-apply preserved). ## Linear [FE-656](https://linear.app/hashintel/issue/FE-656).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/b0d96bc2f0a849d8b57b043bb648a9aa
Stack Context
This PR is the side-chat V1.2 vertical slice under FE-656, stacked on PR #81 (V1.1 — Class 1 Explore: chat anchored to spec items). V1.1 proved the chat seam; V1.2 proves the durable mutation seam — the side-chat can now produce real, persistent changes to a spec.
PR #88 is sized as one coherent slice. V1.2-D (top-bar canonical Apply/Undo), V1.2-E (floating selection menu), and V1.2-F (multi-item pinning) are deferred to follow-up PRs per
memory/PLAN.md.What
End-to-end Annotate flow: the user opens the side-chat on a knowledge item, types a note, hits Save, and it persists to the database. Existing notes for the pinned item render above the chat log as a collapsible
Notessection.The slice lands as three cards plus follow-on UX work:
annotationtable + REST endpoints.PatchListProviderclient module: React provider + hooks for staging and applying patches.Notessection above the chat log.Why
V1.1 vertically proved the chat seam end-to-end (graph → side-chat → SSE → streaming reply). Without V1.2, the side-chat is read-only — Class 4 Annotate, V2 Edit / Drill-down / Propose-edge, and V4 architect-loop emissions all need somewhere to write. V1.2 establishes that surface.
Two design choices worth attention before merge:
D132 — Patch-list module shape
/ln-designexplored four shapes in parallel (minimal-API, plug-in registry, event-log-public, react-context-native). The synthesis takes the React-native public surface (mirrorsSideChatHostexactly) but uses an event-log internal primitive. Rationale:appendPatch(spec, patch[])server primitive lands, migration is "swap the reducer," not "rewrite the public API."D131 — User-driven annotations auto-apply
Originally framed as "uniform staging required for all patch kinds." User feedback during this PR pushed back: re-reviewing what you just typed adds nothing for annotations. Distinction is now explicit in D131:
The patch-list module supports both paths — kind-specific behavior is wired in
SideChatHostvia the auto-applyuseEffect, not in the module itself.Spec / plan deltas
memory/SPEC.md:memory/PLAN.md:Stacked on
ka/fe-656-side-chat, V1.1 Explore) — must merge first.ka/fe-656-side-chat_v4, V4 patch / event-stream data model) — sibling on the same parent, independent of V1.2.Linear
FE-656 — same issue as V1.1 (one Linear issue per frontier item per
CLAUDE.md).Test plan
Automated —
npm run verifypasses (793/793 tests, includes the 9 V1.1-polish tests merged in).Manual walkthrough (in browser, dev server running):
💬on any item row → clickAnnotate→ type Summary + Body → clickSave. Expect: briefSaving annotation…, then✓ Annotation savedwith Undo. New note appears in theNotes (1)section above the chat log.Notes (2)lists both, each independently expandable.Savedisabled until both Summary and Body are non-empty (whitespace-only doesn't count).EscorCancelbutton in composer returns to chat without saving.Annotatebutton is disabled.Notes (N) ›chevron to collapse the entire section. Click individual rows to expand bodies.Retryand× Discard. Restart server →Retrysucceeds.Server sanity:
Known transitional gaps (deferred to follow-ups)
selection_start/selection_endcolumns; V1 leaves them NULL.🤖 Generated with Claude Code