feat(text-editing): Let edits target a specific range of lines - #133
feat(text-editing): Let edits target a specific range of lines#133bezhermoso wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🤖 Comment from Hampton's AI agent (Claude Code), posted on his behalf after discussing the direction with him. Hey Bez — Hampton and I went through both #133 and #134 carefully. Two parts of this work are clearly right and we didn't want to lose them:
The one piece Hampton wasn't comfortable with is exposing So we landed a synthesis in #138 that keeps both of your insights while leaving the public API untouched: the toggle endpoint accepts Would love your review on #138 — especially whether you see holes in the occurrence-mapping logic vs. your resolver-side line box. If it looks right to you, we'd close #133/#134 in its favor. Thank you for the thorough analysis on both PRs; the failure-mode reasoning genuinely shaped the final design. |
Checkbox toggles previously located their target by searching the whole document for the task line's text, so duplicate task lines (or a task whose text prefixes another) made the toggle fail and revert. Worse, the renderer paired rendered checkboxes to source lines by index using a hand-rolled scanner that disagrees with Commonmarker on real constructs (indented fences, ~~~ inside backtick fences, ordered/blockquoted tasks), so a click could silently edit the wrong line. - Renderer: derive each checkbox's source line from Commonmarker's sourcepos metadata, making the parser the single authority on both "renders as a checkbox" and "came from this line". Checkboxes whose source line the toggle endpoint would reject stay disabled. The shared TASK_LINE_PATTERN keeps renderer and endpoint from drifting apart. - Toggle endpoint: accept an optional line param, verify the line's text equals old_text (422 loudly on any drift — line and text must agree), then map the pair to an occurrence ordinal and apply a plain replace_exact. The public operations API is unchanged: no line-based addressing is exposed to agents. - Document the existing replace_section operation in agent instructions as the semantic way to scope edits to one section. The sourcepos technique and the loud-failure design are from Bez Hermoso's PRs #133/#134; this lands them without adding a public line-range qualifier to the operations API. Co-Authored-By: Bez Hermoso <bezalelhermoso@gmail.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#138) Checkbox toggles previously located their target by searching the whole document for the task line's text, so duplicate task lines (or a task whose text prefixes another) made the toggle fail and revert. Worse, the renderer paired rendered checkboxes to source lines by index using a hand-rolled scanner that disagrees with Commonmarker on real constructs (indented fences, ~~~ inside backtick fences, ordered/blockquoted tasks), so a click could silently edit the wrong line. - Renderer: derive each checkbox's source line from Commonmarker's sourcepos metadata, making the parser the single authority on both "renders as a checkbox" and "came from this line". Checkboxes whose source line the toggle endpoint would reject stay disabled. The shared TASK_LINE_PATTERN keeps renderer and endpoint from drifting apart. - Toggle endpoint: accept an optional line param, verify the line's text equals old_text (422 loudly on any drift — line and text must agree), then map the pair to an occurrence ordinal and apply a plain replace_exact. The public operations API is unchanged: no line-based addressing is exposed to agents. - Document the existing replace_section operation in agent instructions as the semantic way to scope edits to one section. The sourcepos technique and the loud-failure design are from Bez Hermoso's PRs #133/#134; this lands them without adding a public line-range qualifier to the operations API. Co-authored-by: Bez Hermoso <bezalelhermoso@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Bez, thank you again for the exceptionally thoughtful work here. Your analysis of the failure modes was exactly right: text alone was not a unique address for duplicate checkbox lines, and the self-verifying line+text pair gives us the failure behavior we want—loud and safe rather than silently editing the wrong place. We ultimately merged #138 as a synthesis of this PR and #134. It keeps that line+text verification inside the checkbox endpoint, maps the verified target back to a standard Since #138 is now on |
What's new
The
replace_exactoperation now accepts an optionallinesqualifier that confines a replacement to a specific line, or an inclusive range of lines. Only occurrences ofold_textthat fall entirely within that range are eligible; if none do, the operation fails with a clear error instead of guessing. Multi-line replacements work as long as the text fits inside the range, and the existingoccurrence,replace_all, andcountqualifiers compose with it — they select among the occurrences inside the range rather than across the whole document.The interactive checkboxes in the plan view now use this: each rendered checkbox knows its source line, and toggling it issues a replacement scoped to that single line.
Examples
Toggle the second of two identical task lines (what the checkbox UI sends):
{ "op": "replace_exact", "old_text": "- [ ] TODO", "new_text": "- [x] TODO", "lines": 4 }Replace text somewhere within a bounded region:
{ "op": "replace_exact", "old_text": "status: draft", "new_text": "status: final", "lines": [12, 30] }Composing with
occurrence— pick the 2nd match within the range, not the 2nd in the document:{ "op": "replace_exact", "old_text": "TBD", "new_text": "Done", "lines": [8, 20], "occurrence": 2 }Composing with
replace_all— replace every match inside the range, leaving matches elsewhere untouched:{ "op": "replace_exact", "old_text": "Q3", "new_text": "Q4", "lines": [40, 55], "replace_all": true }Rationale
Checking a checkbox in the plan view is, under the hood, a request to replace that task's source line (
- [ ] TODO→- [x] TODO). The request carried only the text pair, and the position resolver located the target by searching the whole document for that text. The moment two task lines share the same text — two literal- [ ] TODOitems, a task whose text is a prefix of another (- [ ] Deployvs.- [ ] Deploy to staging), or the same text quoted inside a code fence — the resolver correctly refuses the ambiguous replacement, the request fails, and the checkbox visibly reverts.The root cause is that the toggle request didn't carry enough information to disambiguate: text alone is not a unique address. With this change, the rendered checkbox sends its source line and the resolver boxes the replacement to that line, so duplicate text elsewhere in the document no longer matters.
Why a line box instead of occurrence ordinals alone
The resolver already supports an
occurrenceordinal ("replace the Nth match"), so the plan view could count matches and send an ordinal instead. But that would require the renderer to reproduce the resolver's matching semantics exactly — raw substring matching, including matches inside code fences and non-task text. Any divergence between the two counts wouldn't fail; it would silently toggle the wrong checkbox, which is worse than the bug being fixed.A line box is self-verifying: the resolver independently checks that the requested text actually lives on the requested line. If the client's view is stale or its numbering ever drifts, the result is a loud, safe failure (and the existing revert behavior) — never a misdirected edit. Line and text must both agree for the edit to land.
Other applications
replace_allto one section of a large document — e.g. renaming a term throughout a single section while leaving identical text elsewhere untouched.Agentic AI scenarios
Agents editing plans through the operations API read the document at a known revision, decide on an edit, and submit operations against that snapshot. The
linesqualifier lets an agent pin each edit to the exact region it was reasoning about:replace_allgives agents a cheap way to make bounded bulk edits without enumerating every occurrence.The agent-facing operation instructions have been updated to document the qualifier.
🤖 Generated with Claude Code