FE-673: Side-chat Edit mode — edit, link, and drill-down patches#105
Open
kostandinang wants to merge 40 commits into
Open
FE-673: Side-chat Edit mode — edit, link, and drill-down patches#105kostandinang wants to merge 40 commits into
kostandinang wants to merge 40 commits into
Conversation
Implements V2 of the side-chat patch list per docs/design/SIDE_CHAT.md
(§5.1–5.2, §6.2, §6.3 soft tier): adds `edit`, `edge`, and `drill-down`
patch kinds to the V1 patch-list seam.
Server (edit-impact.ts, edit-route.ts, db.ts, app.ts):
- classifyEditImpact returns `none | soft | hard` based on downstream
count and active-review-set membership.
- PATCH /api/specifications/:id/knowledge-items/:itemId applies edits
for none/soft impact and returns previousContent/previousRationale
for undo. Hard impact returns `updated: false` and defers to V3.
- POST /api/specifications/:id/knowledge-edges/validate validates a
proposed edge against the D125 relation policy.
- POST /api/specifications/:id/knowledge-edges creates a valid edge.
- DELETE /api/specifications/:id/knowledge-edges removes an edge for
the edge applier's undo handler.
- Adds getDownstreamItems, isItemInActiveReviewSet,
updateKnowledgeItemContent, removeKnowledgeRelationship to db.ts.
Client (edit-api.ts, edit-applier.ts, route.tsx, patch-list-reducer.ts,
patch-list-host.tsx):
- Patch-list reducer extended with EditPatch / EdgePatch /
DrillDownPatch using a distributive Omit so the discriminated union
survives StagePatchInput.
- Three applier factories (makeEditApplier, makeEdgeApplier,
makeDrillDownApplier) follow the makeAnnotateApplier shape; each
returns a real undo handler:
• edit's undo re-PATCHes with previousContent/previousRationale.
• edge's undo DELETEs the created edge.
• drill-down throws a clear "not yet implemented in V2" error
rather than logging and faking success — its real D127
detail-focus implementation lands with V3.
- EditItemResponse is a discriminated union: hard-impact responses
don't carry previous values; updated responses always do.
Verification: npm run verify (929 tests, 13 new). Manual playback
deferred (not blocking).
Closes the user-facing piece of revisit/edit mode (Requirement 10) for
the soft-impact case. Hard-impact reshape lives with V3 reading from
FE-697's reconciliation_need queue.
Preserve edit rationale semantics, scope edge deletion to the active specification, and fail edit undo when the restore is deferred. Co-authored-by: Cursor <cursoragent@cursor.com>
Fail edge undo when deletion is rejected and keep knowledge-edge DELETE registration on the shared route helper. Co-authored-by: Cursor <cursoragent@cursor.com>
Use one Zod schema for validate, create, and delete edge mutations so the route contract cannot drift across identical payloads. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the delete edge response assertion aligned with the exported return type so rejected undo paths retain their server reason. Co-authored-by: Cursor <cursoragent@cursor.com>
Two cleanups the V2 review pass missed:
- Bind the structured-list-view scrollTo spy to a variable so the
unbound-method rule no longer fires (latent issue exposed when
V2's schema cascades through the type-aware lint graph; same fix
applied to FE-697 already).
- Drop the stale `specificationId={1}` prop from PatchListProvider
test usages — review commits removed `specificationId` from
PatchListProviderProps but didn't update the test fixtures.
npm run verify: 0 lint errors (was 22), 935 tests pass.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Introduces `chat` as a turn container per docs/design/MULTI_CHAT.md §3. Every specification now has exactly one `kind='interview'` chat; spec creation inserts both atomically. Legacy `turn.specification_id` and `specification.active_turn_id` are retained alongside `turn.chat_id`, `specification.primary_chat_id`, and the mirrored `chat.active_turn_id`, so existing spec-scoped reads keep working through the transition. Migrations 0013-0015 add the new tables and columns, backfilling one interview chat per existing spec and pointing both legacy and chat pointers at the same head. New writes populate both. createTurn rejects a parent that lives in a different chat. advanceHead mirrors the head update to the interview chat. reconciliation_need queue is the next slice on this branch. Verification: npm run verify (lint, fmt:check, typecheck, 663 tests, build).
Introduces `reconciliation_need` per docs/design/MULTI_CHAT.md §3.4 §5: a directed graph signal queue between knowledge_item rows. Each row records `(source_item_id, target_item_id, kind, status)` with optional `reason`, `caused_by_turn_id`, and a nullable `caused_by_patch_id` placeholder for the future patch ledger. A partial unique index gates duplicate `open` rows for the same (source, target, kind) triple while still permitting the same triple to reopen after the previous one resolves. Cascade-on-delete on both knowledge_item FKs keeps the queue honest if items disappear. Migration 0016 creates the table and the partial index. db.ts adds openReconciliationNeed, resolveReconciliationNeed, and listOpenReconciliationNeeds for the lifecycle API. Today's knowledge_item write paths are append-only, so no production fan-out is wired in this slice — that arrives with side-chat V3, the architect loop, or the patch ledger. Together with d99fb74 (chat container) this closes the multi-chat substrate frontier item. SPEC.md A79 and A80 move to validated for the Phase 1 substrate; I111's protecting tests are now real (`chat-substrate.test.ts`, `reconciliation-need.test.ts`). PLAN.md moves the frontier to Recently Completed; the next live frontier is continuous workspace. Verification: npm run verify (lint, fmt:check, typecheck, 672 tests, build).
Adds a Watch entry on the Recently Completed line so the manual outer-loop migration playback against a real `.brunch/` fixture isn't lost. Unit tests cover the migration logic but don't exercise backfill on pre-existing data.
Wraps the spec-head and chat-head updates in a single Drizzle transaction so partial failure can't silently break A79's dual-pointer invariant. If the chat update affects 0 rows (chat row missing despite primary_chat_id being set), advanceHead now throws and the spec-head update rolls back to the previous head instead of committing in isolation. Also adds a one-line schema comment near `reconciliation_need_open_unique` clarifying that omitting specification_id is sound only because knowledge_item.id is globally unique across specs. Verification: npm run verify (673 tests). The new rollback test in chat-substrate.test.ts exercises the missing-chat path with FK checks toggled to set up the corrupted state.
Manual fixture playback per docs/design/MULTI_CHAT.md §8 against a real .brunch/brunch.db (39 specs, 81 turns, 75 knowledge items) revealed that my migrations 0013-0016 collided with FE-656's `0013_annotation` on timestamp 1776300000000. Drizzle's migrate decides "applied" by timestamp alone (not hash), so on the user's DB which already had FE-656's annotation applied, my 0013_chat_table was silently skipped and 0014_turn_chat_id then failed with "no such table: chat". Renumber my migrations to 0014-0017 (timestamps 1776310000000+) so they slot above FE-656's annotation regardless of merge order: - 0013_chat_table → 0014_chat_table - 0014_turn_chat_id → 0015_turn_chat_id - 0015_specification_primary_chat → 0016_specification_primary_chat - 0016_reconciliation_need → 0017_reconciliation_need Re-running migrations against the real fixture: 39 interview chats backfilled (one per spec), 81 turns get chat_id, 39 specs get primary_chat_id, and spec.active_turn_id == chat.active_turn_id holds for every spec. PLAN.md's Recently Completed entry now reflects the playback evidence and the 0014-0017 numbering. Verified: npm run verify (673 tests) plus the fixture playback above. When FE-656 V1 lands in main, this branch slots cleanly above 0013_annotation. If FE-697 lands first, FE-656 V1 will need to renumber its annotation during rebase.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Extends side-chat-prompt.ts additively with:
- SideChatMode type ('explore' | 'edit') and BuildPromptOptions.mode field;
default is 'explore' so existing call sites are unchanged.
- SIDE_CHAT_EDIT_MODE_PROMPT addendum injected when mode='edit', telling
the model to call propose_edit for wording changes / rationale
clarifications and stay conversational otherwise.
- getSideChatTools(mode) factory returning {} for explore, { propose_edit }
for edit.
- proposeEditTool with newContent (required, trimmed, non-empty) and
newRationale (optional) input schema. execute echoes the validated
input — the actual edit applies later through the patch list path.
This is the prompt + tool foundation for V2 chat-driven Edit mode. Wiring
through side-chat-route.ts (mode-aware streamText with tools), the SSE
patch-proposal chunk type, the client stream consumer, and the popover
mode-toggle UI follow as separate commits on this branch.
Inline FE-698 migration TODO: when the shared prompt/tool registry lands
under FE-698, move SIDE_CHAT_EDIT_MODE_PROMPT and proposeEditTool into
that registry and replace the inline definitions with registry lookups.
Test plan: 29 passing in side-chat-prompt.test.ts including 9 new tests
for mode dispatch, tool exposure, and propose_edit input schema.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds optional `mode: 'explore' | 'edit'` to the side-chat request schema
(default 'explore' so existing callers are unchanged). The route:
- Passes mode to buildSideChatPrompt so the edit-mode system addendum
is included when mode='edit'.
- Calls getSideChatTools(mode) and passes the result as `tools` to
streamText. Explore mode passes {} (no tools), edit mode passes
{ propose_edit }.
- Rejects unknown mode values at the request-validation boundary with
a 400.
Loosens getSideChatTools' return type from `as const` literals to a
plain optional-record so it satisfies AI SDK's ToolSet contract.
This still does NOT emit tool-call chunks through the SSE stream — the
existing textStream loop continues to surface only text-delta chunks.
Tool-call surface in the SSE stream and the corresponding client-side
parser land in the next commit.
FE-698 migration TODO unchanged: when the agent-tools substrate lands,
move getSideChatTools and the propose_edit tool definition into the
shared registry; the route will then look up tools by name from the
registry instead of importing from side-chat-prompt.
Test plan: 4 new route tests pass — explore-default no-tools, edit-mode
tools-included, edit-mode prompt-addendum, unknown-mode rejection. All
existing side-chat route tests unchanged. Full suite green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches the side-chat route from result.textStream to result.fullStream
so it can dispatch typed parts:
- text-delta parts continue to emit as `{type: 'text-delta', delta}`
SSE chunks (existing wire format unchanged for V1 / explore mode).
- tool-call parts with toolName='propose_edit' emit a new
`{type: 'patch-proposal', toolCallId, toolName, input}` SSE chunk.
The client patch-list staging path (next commit) consumes this.
- tool-call parts with any other toolName are silently dropped — defensive
guard so unknown tools do not leak through as patch proposals.
- All other fullStream part types (tool-result, finish, etc.) are
ignored; the existing [DONE] sentinel still terminates the stream.
The chunk classifier (`sideChatStreamChunkFromPart`) is local to this
file and uses structural type guards rather than depending on AI SDK's
internal `TextStreamPart` discriminator types — keeps the route loose
to AI SDK version bumps and to whatever the FE-698 agent-tools adapter
ultimately surfaces in the stream.
Test mocks: makeTextStream now exposes both textStream and fullStream
(mapping string chunks to text-delta parts) so existing tests continue
to work without rewriting. New makeFullStream helper supports tests
that need richer chunk types (tool-call, etc.).
Test plan: 23 side-chat-route tests pass (2 new — patch-proposal emission
and unknown-tool defense). Full suite 955 passing. Annotate (V1.x) and
Explore (V1) flows unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the client-side side-chat stream with the V2 surface:
- SideChatMode type ('explore' | 'edit') and SideChatStreamRequest.mode
field. When set, mode is forwarded in the POST body so the server
knows which mode-specific prompt + tools to register.
- ProposeEditInput type for the validated tool-call input shape.
- New SideChatStreamEvent variant: { type: 'patch-proposal', toolCallId,
toolName: 'propose_edit', input: ProposeEditInput }.
- parseSideChatSSEBuffer dispatches the new chunk type and validates:
toolName must equal 'propose_edit', toolCallId must be a string, input
must contain a non-empty newContent string. Unknown / malformed
chunks are silently dropped.
Adds the first test file for the side-chat stream parser
(side-chat-stream.test.ts) covering all four event variants plus
mode propagation.
FE-698 migration TODO: when the agent-tools substrate lands, this client
parser may want to lift toolName validation into a shared registry
client-side too. For V2 we only know about propose_edit; future
propose_edge / propose_drill_down extensions need their own
event-variant additions or a more generic shape.
Test plan: 11 stream-parser tests pass (5 new patch-proposal cases,
3 new mode/forwarding cases, 3 existing-shape regressions).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends SideChatHost with the V2 chat-driven Edit mode hooks:
- Adds `mode: SideChatMode` to the active-side-chat session state, default
'explore' on openFor.
- Exposes `setMode(mode)` on the SideChatContextValue so callers (the
popover toggle, future tests, future external mode switchers) can
flip the active mode.
- Forwards `mode` to streamSideChatResponse only when non-explore so V1
/ annotate request payloads stay byte-identical.
- Handles 'patch-proposal' stream events by staging an EditPatch through
patchList.stage with the session's anchor + the validated input. Uses
a patchListRef to avoid taking patchList as a useCallback dep, which
would re-create submitMessage on every patch-list state change and
remount the popover composer mid-stream.
- Adds a small summarizeEditContent helper to populate the
patch-list overlay preview ("Edit: <content>...").
Annotate (V1.x) and Explore (V1) flows are unchanged — the only divergence
is the optional `mode` field in the request body.
Test plan: 28 host tests pass, including 3 new tests for V2 — mode='edit'
forwarded, mode omitted by default, EditPatch staged on patch-proposal
arrival with correct anchor + content + rationale.
FE-698 migration TODO unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds optional `mode` and `onModeChange` props to SideChatPopover; when
both are provided, the Edit button becomes an active aria-pressed toggle
that flips the host between 'explore' and 'edit'. When onModeChange is
absent (legacy callers), the button keeps its disabled "coming in V2"
stub so existing render contracts stay backwards-compatible.
Visual states:
- mode='edit' → blue accent fill (consistent with existing primary
actions in the design tokens), aria-pressed='true'.
- mode='explore' → neutral white surface matching the Annotate button,
aria-pressed='false'.
- onModeChange absent → disabled greyed stub (legacy behavior).
Wires the host: SideChatHost passes `mode` (from session state) and
`onModeChange` (= setMode, only when patchList is present, mirroring
the onAnnotateRequest gate). This closes the V2 chat-driven Edit mode
end-to-end:
Edit toggle → setMode('edit') → submitMessage forwards mode='edit'
→ server registers propose_edit tool → LLM emits tool call
→ SSE patch-proposal chunk → client parser → host stages EditPatch
→ top-bar `N Edits` overlay surfaces it for Apply/Undo.
Test plan: 6 new popover tests cover button enabled/disabled, click
toggles 'explore' ↔ 'edit', aria-pressed reflects mode. Full suite 975
passing. Existing popover and host tests unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR SummaryMedium Risk Overview Introduces a new sticky Updates patch application behavior and UX: Reviewed by Cursor Bugbot for commit 444891c. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: This PR completes Side-chat V2’s chat-driven Edit mode trigger, enabling the UI to request “edit” behavior and receive structured edit proposals back from the model. Changes:
Technical notes: Edit mode is only sent over the wire when non-default ( 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
Three fixes from manual playback (PR #105 review): 1. Popover never showed staged edit patches. The host's stagedForActive filtered useStagedPatches by kind:'annotate' only, so EditPatches landed in patchListState.staged correctly but were invisible in the popover's inline patch list. Auto-apply doesn't trigger for edits (correct), and there's no top-bar overlay yet (V4 territory), so the user saw no UI feedback at all when the LLM proposed an edit. Fix: drop the kind filter, map each patch's actual kind through to stagedSummaries, and broaden canUndoForActive to include edit-kind patches. Update popover copy from "pending annotation(s)" to kind- neutral "pending change(s)" / "Staged changes". 2. Race condition when toggling Edit then immediately submitting. setMode updated activeSideChat (state) but activeRef.current was only mirrored via useEffect, which fires after render. Clicking Edit + pressing Enter in the same act read stale mode='explore' from activeRef, so the request went out without mode='edit'. Fix: sync-update activeRef.current alongside the setActiveSideChat call inside setMode. 3. Disabled Edit-button copy was V2-stub-specific. Replaced "Edit (coming in V2)" / "Edit — coming in V2" with "Edit unavailable" / "Edit unavailable in this context" so the stub is meaningful regardless of which V is shipping. Test plan: - 84 client tests pass across host + popover suites including 4 new regression tests: - sends mode='edit' when message is submitted immediately after toggling Edit - disabled Edit button keeps aria-label "Edit unavailable" - disabled Edit button title is "Edit unavailable in this context" - popover surfaces staged edit patches alongside annotate - npm run verify: 1000 tests passing, build succeeds - Manual fixture playback: edit-mode propose_edit → patch staged → visible in popover → Apply runs makeEditApplier → soft-impact updates content Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…it apply Surfaced from manual playback (PR #105): 1. Apply button was labeled "Retry" — annotate-flow language assuming the manual button is only used as a retry-after-failure path (annotates auto-apply on first try). For edits the button IS the primary action, not a retry. Renamed to "Apply". 2. Saving status said "Saving annotation…" — now "Saving change…". 3. Saved-toast said "Annotation saved" — now "Change saved". 4. Discard aria-label said "Discard staged annotation: ..." — now "Discard staged change: ...". 5. Edit-applier didn't invalidate the entity query cache after a successful apply, so the goal/decision/etc. content displayed in the structured-list and graph view stayed stale until the user navigated away and back. The user clicks Apply, sees "Change saved" toast, but nothing visibly changes on the page — confusing. makeEditApplier now invalidates both the active-path entities and project-wide entities query keys after apply and after undo. Test plan: 1000 tests passing (test copy assertions updated to match the new labels). Manual playback after these fixes: edit a soft-impact goal in the running app → patch staged → click Apply → goal text visibly updates in place → "Change saved" toast appears → click Undo → original text restored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2 tasks
Co-authored-by: Cursor <cursoragent@cursor.com>
…2-edit-mode # Conflicts: # memory/PLAN.md # memory/SPEC.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
This was referenced May 8, 2026
Use replaceAll so multi-underscore EdgeRelation values render correctly in staged patch summaries. Co-authored-by: Cursor <cursoragent@cursor.com>
- Add PatchListOverlayBridgeProvider from SideChatHost so the top bar calls the same applyStagedForActive path as the popover; disable Apply when only other items have staged patches. - Import EditImpactTier from patch-list-reducer in side-chat-stream and re-export for callers. - Extend patch-list-overlay tests for bridge behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced May 8, 2026
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 088643a. Configure here.
When the server returns updated: false for hard-impact edits, the pinned
item's underlying content was never mutated. The post-apply refresh loop
unconditionally re-pointed pinnedItem.content to patch.newContent, so the
side-chat header showed the proposed content while the source kept the
old value. Undo could not fix this either: deferred metadata only has
{deferred, impact, message}, so revertedContent stays undefined.
Check lastBatchAppliedMeta for the deferred flag and skip the snapshot
refresh in that case, matching the same flag used by lastApplyDeferred.
kostandinang
force-pushed
the
ka/fe-673-side-chat-v2-edit-mode
branch
from
088643a to
444891c
Compare
6 tasks
kostandinang
changed the title
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
1 participant
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.
What
User-facing side-chat V2 — chat-driven Edit mode covering all three patch kinds (
edit,edge,drill-down), the canonicalPatchListOverlay, hard-impact deferred banner, and per-patch impact chip.PR #97 shipped the server/reducer plumbing; this ships the surface that exercises it.
Linear: FE-673 · Loom
Stacked on #96 for V3 ergonomics only — V2 builds cleanly off
main.End-to-end paths
What ships
mode; pre-classifies edit impact; emitspatch-proposalSSE chunks.ActiveSideChat.mode(explore | edit) persists per pinned item; resolves edge target refs; refreshes pinned content after apply/undo.PatchListOverlay:N pending changes · Apply · Undo+ amber hard-impact deferred banner.Out of scope (V3)
Hard-impact cascade preview, real drill-down behavior, Open-phase Edit router, three-mode segmented control, FE-698 tools registry migration.
Test plan
[Soft]chip → Apply updates both surfaces; Undo restores[Hard — V3]chip → Apply → amber banner; auto-hides[No impact]chipmode