githubnext/chopinPublic

Let's compose rich plans together

Comment on the plan, and accept the thread as a decision #1

MergedKrzysztof-Cieslakmerged 16 commits intomainfromrefs/pull/1/head
  1. KC

    Select part of the plan, comment on it, and reply to each other in the right sidecar. Anyone can **accept** a thread — it freezes read-only, becomes an anchored decision projected into `plan.mdx`, and starts an agent turn to revise the prose — or **dismiss** it, which closes it without the agent.

    Sixteen commits. The first eight build the feature, one per phase, each independently green; the rest are fixes and refinements found by running it, several in code that predates this branch. **375 tests**, up from 230.

    ## How it hangs together

    **The record owns the thread; the plan shows the accepted ones.** A thread lives in `data/<room>/state.json` beside the questionnaire records, not in the Yjs document — a comment must not be undoable with Ctrl-Z, and the agent must not be able to rewrite what somebody said about its work. On accept, and only then, a `<Decision>` node is appended so the source carries the decision when read alone.

    **A passage is an anchor one level finer.** An anchor names a block, which is the right grain for a decision and the wrong one for a remark about one sentence. So a passage is the block run plus a range inside it, held twice over: Yjs relative positions, so the highlight stretches as somebody types inside the phrase, and the quoted text, which finds it again when they cannot resolve. Same recovery ladder as an anchor, same refusal at the bottom — two occurrences equally near the recorded offset recover neither.

    **An accepted thread points at what it produced, not at what it was about.** The prose a thread marked is usually the prose it asked to have rewritten, so after the agent acts the subject is gone by design. What was discussed is frozen as a quote; where the decision now lives is the result anchor.

    **Accepting is an address.** The `@ai` rule separates conversation from instruction, and a button press is already the latter, so accept starts a turn through the same queue a message does, with a system line in the transcript explaining why the agent began moving.

    **A client sends what it read, not where it thinks that is.** `comment:start` carries block indices and the selected text; the server finds the quote and mints every Yjs position itself. Finding it *is* the concurrency check, and a better one than a digest — it tests whether the sentence is still there, and a browser cannot compute a canonical block digest without re-serialising the document.

    ## Decisions that changed while building

    - **A note's text is an attribute, not children.** A flow component's children parse as blocks, so prose between the tags comes back wrapped in a paragraph and flattens on export — which `room.validate()` rejects the whole plan for. Attributes round-trip newlines, quotes and angle brackets exactly, and the delimiter is escaped, so a note cannot break out into a second component. - **No block digests on `comment:start`**, for the reason above. - **Provenance lives on the container, not the answer.** `<Questionnaire by at>` rather than `<Answer by at>`: a questionnaire resolves as a unit, so per-answer provenance would write one handle and one moment N times. `<Decision by at>` already works this way.

    ## Bugs found and fixed

    Four were in code that predates this branch. The error surfacing added here is what made two of them visible at all.

    | | | |---|---| | **`AGENT=off` opened a session anyway** | `chat:send` was gated; a button press was not. Found by running the server. | | **Accept rolled back after the document had changed** | A failed relay left a `<Decision>` in the plan whose record said the thread was open — and the next accept would add a second node with the same id. | | **The plan never opened if the socket was still shaking hands** | *Pre-existing.* `connect()` ran once on mount and `ask` refused; nothing retried, so the editor stayed locked with the chrome saying "Loading". This was the bug reported as "sometimes I can't reload". | | **No re-sync after a reconnect** | *Pre-existing.* `#replay()` is reachable only from `#open()`, so a client came back unlocked over a document quietly missing whatever arrived while it was away. | | **A comment could take the room down** | Resolving anchors ran between applying the document and declaring it synced, inside a promise nobody caught. Now it runs after, guarded. | | **`Wire.#receive` fanned frames out unguarded** | *Pre-existing.* The first handler to throw silenced every handler after it for that kind. | | **`ThreadStore.refresh` dropped dismissed threads in two branches** | The test only covered one; removing the check from the other left it green. | | **An invisible palette, twice** | Washes calibrated against a value that was doing nothing — the old rule's visible part was an outline. Decided prose rendered `#f2f9ff`. |

    ## Adjacent fixes, deliberately in scope

    **Rebasing only ever happened inside `edit_plan`**, so a block somebody moved kept its anchors broken until the agent next edited, and everything restored from disk was expressed in a history the new document did not have. It now runs on the snapshot debounce, on an epoch rotation, and before `open` returns — and broadcasts only when the snapshot actually moved. This closes the identical pre-existing gap for question anchors.

    **A failed open now says so** instead of claiming to still be loading, which is the difference between a bug somebody can report and one they cannot.

    ## One vocabulary for the sidecar

    Questions and comments used to look nothing alike: one outlined a block through a DOM attribute, the other washed a range; one had a header and no provenance, the other provenance and no header.

    Now both draw through one `SidecarCard`, and one `CSS.highlights` registration marks prose — **only while the reader is pointing at a card**. Standing marks were tried and taken out: a plan accumulates decisions, so anything permanent ends up covering most of the document.

    `::highlight()` accepts neither outline nor box-shadow, which is why the wash is what both could be.

    ## Risk

    The forward half of the Yjs position arithmetic is not exported by `@lexical/yjs`, so it is repeated in `plan/room.ts`. `passages.test.ts` pins it — including the test proving a highlight *stretches* when you type inside the phrase, which is the whole justification for keeping relative positions.

    ## Testing

    **230 → 375.** Beyond the unit level: service ordering and relationship bookkeeping against a real `Service.open` room; `Chat.instruct`'s queue, ceiling and `AGENT=off` gate; eleven socket-level tests against a spawned server; the client store's reducers; and a colour test that composites each highlight over the page and refuses anything indistinguishable from it — there is no DOM in the test runtime, so a mark painted white looks exactly like one that failed to paint.

    **Every test was verified by breaking the code it covers.** That is how the `refresh` duplication and an over-narrow guard were found. The whole path was also driven by hand against a running server: two members, marking, replying, typing, accept, dismiss, stale-index refusal, restart recovery, and provenance reaching `plan.mdx`.

    `DEV_COMMENTS=1` marks a real phrase on room open, so the server path is exercisable with no client and no agent.

    ## Known and deliberate

    - A questionnaire answered before this branch shows no footer — its record has a resolver but the file has nothing and there is no timestamp to recover. Not backfilled on load: the plan is a file a person can edit between runs, and correcting it at startup is what we decided against for missing `<Decision>` nodes. - Nothing marks the prose at rest, so the sidecar is the only place you learn a passage has a comment. If that matters, a gutter marker is the cheap middle ground. - `--radius-sm` and `--radius-md` are used five times in `styles.css` and defined nowhere, so those `border-radius` declarations are silently dropped. Pre-existing, untouched.

    ## Not included

    Editing or deleting a posted note, reactions, threads on the chat transcript, per-card vertical alignment to the anchor.

  2. Krzysztof-Cieslakhead-ref-force-pushed 8328c88cc3be3 months ago on GitHub

  3. Krzysztof-Cieslakmerged 1124949eb1fe3 months ago on GitHub

  4. Krzysztof-Cieslakclosed3 months ago on GitHub

  5. MaggieAppletonreferenced this in Let Tab leave the editor outside lists #292last month on GitHub

  6. MaggieAppletonreferenced this in Replace the native link prompt with an inline link popover #298last month on GitHub