FE-697: Decouple turns from specifications#96
Merged
Conversation
kostandinang
changed the title
This was referenced May 6, 2026
Contributor
Author
7 tasks
lunelson
changed the base branch from
ln/fe-692-planning-groundwork
to
graphite-base/96
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.
kostandinang
force-pushed
the
ka/fe-697-decouple-turns-from-specs
branch
from
eb13579 to
df11855
Compare
kostandinang
force-pushed
the
graphite-base/96
branch
from
9f3f4ec to
72bf75f
Compare
kostandinang
changed the base branch from
graphite-base/96
to
ka/fe-673-side-chat-v2
5 tasks
PR SummaryMedium Risk Overview Updates server write paths so spec creation inserts the spec + interview chat in one transaction, Adds a Reviewed by Cursor Bugbot for commit a4a5f4b. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: Implements Phase 1 of the multi-chat substrate by introducing durable chat containers and a directed reconciliation-need queue, while keeping legacy spec-scoped turn pointers for a safe transition. Changes:
Technical Notes: Adds edit-impact classification based on downstream edges / active review sets, and expands test coverage for chat substrate, reconciliation needs, and edit/edge lifecycle. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
8 tasks
Co-authored-by: Cursor <cursoragent@cursor.com>
lunelson
approved these changes
May 8, 2026
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.
Implements
docs/design/MULTI_CHAT.md§3 Phase 1: turns are now anchored to a newchatcontainer instead of hanging directly offspecification, and areconciliation_needqueue records directed open issues between knowledge items. No user-visible change in this slice.Substrate
chattable — onekind='interview'chat per spec, plus future side-chat slots;active_turn_idmirrors the legacy spec head.turn.chat_id, populated on every new write; legacyturn.specification_idretained during transition.specification.primary_chat_idset transactionally with spec creation; legacyspecification.active_turn_idretained.reconciliation_needtable with directedsource_item_id/target_item_id, narrowkind(supersedes/needs_confirmation) andstatus(open/resolved), partial unique index gating duplicate open rows per(source, target, kind), cascade FK on knowledge_item, pluscaused_by_turn_idand a nullablecaused_by_patch_idplaceholder for the future patch ledger.Application changes
createTurnpopulateschat_idfrom the spec's primary chat and rejects aparent_turn_idwhose chat differs (parent-chat consistency invariant).advanceHeadupdates spec head and chat head in one transaction; throws and rolls back if the chat update affects 0 rows, protecting A79's dual-pointer invariant against partial-failure drift.openReconciliationNeed/resolveReconciliationNeed/listOpenReconciliationNeedsadd the lifecycle API. Today'sknowledge_itemwrite 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.Migration ordering
Migrations are numbered
0014_chat_table→0017_reconciliation_need(timestamps1776310000000+) so they slot above FE-656's0013_annotationregardless of merge order. If FE-656 V1 lands in main first, this branch slots cleanly above. If FE-697 lands first, FE-656 V1 will need to renumber its annotation during rebase.Canonical reconciliation
memory/SPEC.mdA79 and A80 move tovalidatedfor the Phase 1 substrate.memory/PLAN.mdmoves the multi-chat-substrate frontier fromActivetoRecently Completed; the next live frontier is continuous workspace.Unblocks
reconciliation_needinstead of ad-hoc REVISIT state.chattable.Verification
npm run verify— 0 lint, fmt, typecheck errors; 673 tests; build succeeds.docs/design/MULTI_CHAT.md§8 against a real.brunch/brunch.db(39 specs, 81 turns, 75 knowledge items): all backfilled correctly, dual-pointer equivalence holds for every spec. The playback caught the FE-656 timestamp collision that unit tests would have missed.Watch
Legacy
turn.specification_idandspecification.active_turn_idremain alongside the new chat pointers. Cleanup migration is deferred until callers read ownership throughchat_id.Test plan
.brunch/fixture playback after rebasing onto a current main