Skip to content

Milestone Editing#974

Open
Luke-Bilhorn wants to merge 44 commits into
mainfrom
milestone-editing
Open

Milestone Editing#974
Luke-Bilhorn wants to merge 44 commits into
mainfrom
milestone-editing

Conversation

@Luke-Bilhorn

@Luke-Bilhorn Luke-Bilhorn commented May 18, 2026

Copy link
Copy Markdown
Contributor

Expands the subdivision editing PR to include milestone editing, with associated tests

Test Checklist

1. Setup & Migration

Deleted; this migration was not for this Milestone PR

2. Interface Settings panel

  • Open Interface Settings → "Pagination & Subdivisions" section renders.
  • Cells per page: enter a valid value (e.g. 25) → persists; reopening editor repaginates milestones to 25/page.
  • Cells per page clamping: enter 3 → clamps to 5 on commit; enter 250 → clamps to 200; enter non-numeric → reverts to last valid; Enter key commits.
  • Maximum subdivision length toggle OFF → stored as 0 (resolver falls back to cellsPerPage as the threshold).
  • Maximum subdivision length toggle ON → number input appears, pre-filled (~cellsPerPage × 2); clamps to 1–5000; invalid reverts.
  • Enable Milestone Placement Editing toggle ON/OFF → change is reflected live in an open editor's accordion (structural controls appear/disappear without reload).
  • Change a setting from the VS Code Settings UI (not the panel) → Interface Settings panel and open editors stay in sync.
  • useSubdivisionNumberLabels is currently hidden in the Interface Settings panel — verify it can still be toggled via VS Code settings.json and takes effect (see Display section).

3. Milestone display & pagination

  • Milestone accordion lists all milestones with correct labels ("Book Chapter", e.g. "Isaiah 1") and text/audio progress indicators.
  • A milestone with no custom breaks paginates arithmetically into cellsPerPage-sized subdivisions (default 50 → "1–50", "51–100", …) — matches legacy pagination exactly.
  • A milestone shorter than cellsPerPage shows a single subdivision (no chunking).
  • With maxSubdivisionLength set (e.g. 200) and a long stretch between breaks (e.g. 300 cells): the stretch sub-chunks into cellsPerPage-sized pages; a stretch shorter than the threshold stays unsplit.
  • Clicking a subdivision navigates to / loads that page of cells.
  • Navigation header reflects the correct milestone/subdivision while paging.

4. Subdivision label preference (useSubdivisionNumberLabels)

  • Default (false): a named subdivision shows its name, with the numeric range as a muted suffix.
  • Set true: subdivisions always show the numeric range as the primary label even when a name exists.
  • Toggling the setting updates open editors live (no reload) and does not repaginate.

5. Settings/gear gating & source-only rules

  • With placement editing OFF: the gear toggle reveals only rename pencils (milestone + subdivision); no add/remove/promote/demote controls.
  • With placement editing ON: the gear reveals add-milestone, remove (trash), promote (↑), demote (↓) controls.
  • Gear toggle hides all edit affordances when off; closing & reopening the accordion collapses settings mode back to default.
  • On a target (.codex) document: "Add break", "Add milestone", delete-subdivision, promote/demote, and reset controls are not shown; milestone + subdivision rename pencils still appear.
  • Provider rejects any structural/subdivision write that originates from a target document (defense-in-depth — even if UI is bypassed, a warning is shown and nothing changes).

6. Milestone rename

  • Click the milestone pencil → inline input replaces the title, text auto-selected.
  • Save with a valid changed value → label updates immediately (optimistic), persists, and mirrors to the target milestone.
  • Save button disabled when value is empty, whitespace-only, or unchanged; whitespace is trimmed on save.
  • Enter saves, Escape reverts (revert sends no message).
  • Milestone rename works on both source and target documents (renaming is allowed on target; it does not mirror back to source).
  • Source rename overwrites a target-side rename (source is authoritative — no target "stickiness").

7. Subdivision rename

  • Rename pencil appears only for subdivisions that carry a key (custom or auto-chunk with a cell anchor).
  • Saving a name posts updateMilestoneSubdivisionName; the name displays and the numeric range stays visible.
  • Naming an auto-chunk on the source promotes it into a persistent custom placement.
  • Naming the implicit first subdivision does not create a placement (stored under __start__).
  • Clearing a name (empty string) leaves the placement intact (does not delete the break).
  • Unchanged name → no message posted.
  • Focus is retained in the rename input across parent re-renders (regression: focus loss was fixed) — type, let a re-render happen, keep typing.
  • Target-side rename never creates a placement; target name overrides source name locally, but an empty target override falls back to the source-mirrored name.

8. Add subdivision break (source only)

  • "Add break…" button shows on source when the milestone has ≥2 cells; hidden when the milestone has only 1 cell.
  • Enter a valid cell number (2..last) → new break created at the Nth root cell; existing source names preserved.
  • Out-of-range or empty input → inline error ("Enter a number between 2 and N." / "This milestone is too short to split."), nothing posted.
  • Re-adding a break at the same cell is idempotent (no duplicate).
  • Cancel/Escape closes the form without posting.
  • The new break mirrors to the paired target.

9. Reset subdivisions (source only)

  • "Reset to default breaks" appears only when custom breaks exist.
  • Reset requires two clicks (arm-then-confirm, auto-disarms after ~3s); second click clears all custom breaks and restores arithmetic pagination.
  • Reset mirrors to the target.

10. Add milestone at cell (split) — placement editing ON, source only

  • "Add milestone…" appears only with the feature on, source, settings mode.
  • Enter a valid cell number (≥2) → a new milestone is inserted there, splitting the original; placements before/after the boundary are redistributed correctly; a placement exactly at the boundary becomes the new milestone's first-subdivision name.
  • New milestone with no inherited name gets the "New milestone" placeholder label.
  • cellNumber < 2 is rejected (no-op).
  • The insertion + redistributed subdivisions mirror to the target by UUID.
  • After the split, the editor repositions correctly (see Position restoration).

11. Promote subdivision → milestone (placement editing ON, source only)

  • Promote (↑) appears on custom subdivisions that are not the first subdivision.
  • Single click promotes: the subdivision's name becomes the new milestone's label; the source placement at that anchor is removed (it's now a milestone).
  • An unnamed promoted subdivision yields the "New milestone" placeholder.
  • The first/implicit subdivision cannot be promoted (control hidden; provider refuses).
  • Promotion mirrors to the target; if the translator had locally named that seam on the target, the target milestone keeps the local name.

12. Demote milestone → subdivision (placement editing ON, source only)

  • Demote (↓) appears on every milestone except the first.
  • Single click demotes (no confirm): the milestone merges into the previous milestone, and the boundary is preserved as a custom subdivision carrying the demoted milestone's label.
  • First milestone never shows demote (or remove).
  • Demotion mirrors the merge + boundary onto the target; target translator's subdivision names from the removed milestone fold onto the survivor.

13. Remove milestone (placement editing ON, source only)

  • Remove (trash) appears on every milestone except the first (first shows a disabled/ghost trash).
  • Remove requires two clicks (arm-then-confirm within ~3s, tooltip explains "content merges into the previous milestone").
  • After removal: content cells merge into the previous milestone; subdivision breaks inside the removed milestone are lifted onto the survivor; the boundary itself is not preserved (contrast with demote).
  • First milestone cannot be removed (UI disabled; provider refuses with a warning).
  • Removal mirrors the soft-delete onto the target by UUID.

14. Delete a single subdivision break (source only)

  • Trash icon appears only on custom subdivisions (auto/first show a disabled ghost icon).
  • Single click removes just that break (posts the filtered placement list); pagination recomputes; mirrors to target.

15. Source → target mirroring (cross-cutting)

  • For each operation above, open the paired source and target side-by-side and confirm the target reflects: renames, added/removed breaks, splits, merges, promotes, demotes.
  • Mirroring is by UUID and is skipped (no target change, no crash) when the source and target milestone roots have diverged (e.g. target edited independently). Verify a divergent case doesn't corrupt or delete misaligned target content.
  • Target document does not need an open webview for the mirror to apply (background open path) — verify after closing the target, editing source, then reopening target.

16. Text editing ↔ milestone interactions (via TextCellEditor)

Matt and I decided we did not need to test it rigorously—it works.

  • *Start editing a cell's text (dirty state) → in the milestone accordion, subdivision rows become dimmed / not-clickable and the "Save changes first to change section" banner appears.
  • With unsaved changes, click a subdivision (or another milestone) → navigation is blocked (no page change); confirm no requestCellsForMilestone fires.
  • Save the edit → subdivision/milestone navigation re-enables and works normally.
  • After saving a text edit inside a subdivision, the editor stays on the same milestone/subsection (pagination cache rebuilds without throwing you to a different page) and progress indicators update.
  • Edit & save text in a cell within a custom-broken milestone → the custom breaks and their names are preserved (cache invalidation rebuilds subdivisions, doesn't reset them to arithmetic).
  • Add a new content cell via the editor → the root-cell count grows: "Add break at cell…" max/range reflects the new count, existing cell-ID-anchored breaks stay put, and a trailing auto-subdivision extends/appears as expected.
  • Delete (soft-delete) a content cell that is a subdivision's anchor (startCellId) → the stale break is silently pruned and pagination falls back gracefully (no empty/broken subsection).
  • Confirm milestone cells themselves are not editable as normal text in TextCellEditor (no value/edit affordances for CodexCellTypes.MILESTONE); their label is changed only via the accordion rename.
  • On a target document: editing/saving cell text does not alter source milestone structure; the unsaved-changes gate applies independently per open document.
  • Edge case — hold unsaved text edits, then trigger a forced milestone refresh (e.g. a source-side structural edit mirrors to this target) → verify the unsaved edit is handled sanely (not silently lost without warning) and the view repositions correctly.*

17. Staying in the right place after edits and navigation

  • After you promote, demote, add, or remove a milestone, the editor automatically jumps to and shows the correct section — you're never left staring at the wrong chapter, the previous spot, or a blank page.
  • deleted
  • When you first open a file, it opens to the expected section/page.
  • After an edit moves you to a new section, the view stays put — it doesn't flicker, bounce back to the old spot, or freeze.

18. Persistence & reload

  • After each edit type, save and reopen the file (and the project): changes persist on disk in the milestone cell's metadata.data (subdivisions / subdivisionNames / subdivisionNamesFromSource), and the milestone/subdivision structure renders identically.
  • Edits survive a git sync / 3-way merge without losing the milestone label (initial-import edit anchors the label).
  • cellsPerPage / maxSubdivisionLength changes invalidate the pagination cache and repaginate on next view.

19. Regression / smoke (non-milestone functionality unaffected)

  • Deleted; there is no migration
  • Cells with parentId (children), paratext, and soft-deleted cells are correctly excluded from root-content-cell counting (so break-at-cell numbering matches what the user sees).
  • A document where all milestones collapse to one "virtual" milestone (no real milestone cells) still paginates and renders.

Introduce a dedicated subdivision model that lets milestones describe
custom rendering breaks, while preserving today's arithmetic 50-cell
chunking as the default fallback. Pagination, subsection lookup, and
subsection progress now all go through a single resolver, so future
writes to `metadata.data.subdivisions` immediately change what the
editor renders without touching any other code path.

- Types: add `MilestoneSubdivisionPlacement`, `SubdivisionInfo`, and
  `metadata.data.subdivisions` / `metadata.data.subdivisionNames` on
  milestone cells; extend `MilestoneInfo` with resolved `subdivisions`
  and add `updateMilestoneSubdivisions` / `updateMilestoneSubdivisionName`
  editor messages for upcoming writers.
- Resolver (`utils/subdivisionUtils.ts`): pure function that returns
  ordered, non-overlapping subdivisions for a milestone. Handles
  arithmetic fallback, stable ordering, anchor pruning on deleted cells,
  duplicate collapse, and target-side name overrides via
  `FIRST_SUBDIVISION_KEY`.
- CodexCellDocument: `buildMilestoneIndex`, `getCellsForMilestone`,
  `getSubsectionCountForMilestone`, `findMilestoneAndSubsectionForCell`,
  and `calculateSubsectionProgress` now share the resolver. Behavior is
  byte-identical for documents without custom subdivisions.
- Tests: add unit + integration coverage for the resolver and the
  document APIs (custom breaks, slicing, stale anchor pruning, legacy
  50-cell behavior).

Made-with: Cursor
Wire up `updateMilestoneSubdivisions` and `updateMilestoneSubdivisionName`
editor messages so the UI can now persist custom subdivision breaks and
names. Placements are source-authoritative — edits from a `.codex`
document are rejected and mirrored onto the paired source's target only
after a root-cell-ID sanity check confirms the two milestones line up.
Names live on a separate `subdivisionNames` map per document so source
and target can be renamed independently.

- Message handlers: validate anchors against current root content cells,
  strip names on mirror (target uses its own override map), recover
  gracefully when the paired document diverges, and refresh the webview
  via the existing milestone-refresh path.
- Provider helpers: `getPairedNotebookUri` and
  `getOrOpenDocumentForUri` keep the paired-document plumbing localized
  to the provider and reuse any already-open `CodexCellDocument`.
- Document: expose `getRootContentCellIdsForMilestone` for anchor
  validation / pairing checks, and invalidate the milestone-index cache
  whenever `subdivisions` or `subdivisionNames` change so renames and
  break edits appear on the next render.
- Tests: cover anchor-set extraction (including paratext / child /
  deleted exclusions), cache invalidation on both subdivisions and name
  overrides, and the full round-trip from placements → arithmetic
  fallback when placements are cleared.

Made-with: Cursor
The editor now prefers provider-computed subdivisions when building the
UI's `Subsection` list, so custom milestone breaks and their names flow
through to navigation the moment the resolver emits them. The prior
arithmetic calculation remains as a pure fallback for the brief window
before the first `milestoneIndex` lands (or for any stale payloads
missing `subdivisions`), which preserves today's behaviour exactly for
notebooks without custom breaks.

- Extract the subsection-building logic into
  `CodexCellEditor/utils/subdivisionUtils.ts` so it can be unit-tested
  independently and reused by future tail-append / renaming paths.
- Extend the `Subsection` type with `name`, `key`, `startCellId`, and
  `source` so downstream UI (accordion, header) can display names, tell
  custom breaks from auto ones, and echo the stable key back to the
  provider on renames.
- Add unit tests covering empty milestones, subdivision preference,
  arithmetic fallback parity, ID stability, and edge-case page sizes.

Made-with: Cursor
Each keyed subsection in the milestone accordion now carries a hover
"rename" affordance. Editing is inline, works on both source and target
documents (names are stored per-document in `subdivisionNames`), and an
optimistic local cache keeps the new name on screen while the provider
refreshes. Numeric ranges remain visible alongside the name so users
never lose track of where a subdivision starts and ends.

- Skip the affordance entirely for legacy subsections without a `key`
  (no resolver payload, nothing to persist) so the UI degrades cleanly.
- Clearing the input posts an empty `newName`, which the provider
  handler interprets as "remove override" and falls back to the numeric
  range.
- Pressing Enter saves, Escape cancels; cancel never posts a message.
- Add focused tests covering affordance gating, display behaviour, the
  save/clear/no-op paths, and cancel.

Made-with: Cursor
Source-side authors can now remove an individual custom subdivision
break and, if they want to start over, reset all custom breaks in a
milestone back to the default arithmetic chunking.

Both actions post `updateMilestoneSubdivisions` with a recomputed
placement list derived from the resolver's subdivisions (custom-index-
>0 with a stable startCellId). The provider already mirrors the new
list to the paired target document, so source and target stay aligned.

- The × button only appears for subdivisions whose `source === "custom"`
  and that carry a `startCellId`, so legacy payloads and the implicit
  first subdivision are untouched.
- "Reset to default breaks" uses an in-place two-click confirmation
  (arm-then-commit, auto-disarming after 3s) to avoid accidental loss
  without pulling in a modal. The accessible label flips between
  "Reset Subdivisions" and "Confirm Reset Subdivisions" so assistive
  tech announces the armed state.
- Neither control is rendered on target documents; placements are
  source-authoritative and the provider rejects writes from target
  URIs anyway — the UI just omits the affordance up front.
- Extend the lucide-react test mock with the new X/Undo2 icons and add
  focused coverage for the control visibility and both post shapes.

Made-with: Cursor
Extract the save/mirror/refresh pipeline from updateMilestoneSubdivisions
into commitMilestoneSubdivisionPlacements so a second entry point — a
source-only add-break command that takes a 1-based cellNumber — can reuse
the same source-authoritative guarantees (root-cell validation, target
mirroring with names stripped, divergence-skip on root mismatch).

Tests exercise the new handler via a test-only export of the handler map,
covering the happy path, the preserve-names-on-append path, out-of-range
rejection, idempotence on re-add, and the non-source rejection guard.

Made-with: Cursor
…nly)

Source-side users can now split a milestone by typing the target cell
number directly into the accordion footer instead of having to touch the
underlying data. The form validates the number against the milestone's
root-cell range before posting addMilestoneSubdivisionAnchor, surfaces
an inline error on out-of-range input, and respects Escape/Cancel to
close without committing. Hidden entirely on target documents and on
milestones with fewer than 2 cells.

Made-with: Cursor
…bels

Introduces codex-editor-extension.useSubdivisionNumberLabels (default false).
When enabled, the milestone accordion always shows the numeric cell range
instead of a user-assigned subdivision name. The provider broadcasts the
current value on initial pagination load and whenever the setting changes.

Made-with: Cursor
…ings

Adds a Pagination & Subdivisions section to the Interface Settings webview
so users can adjust the default milestone page size and toggle whether
subdivisions render as numeric ranges instead of their names. The panel
listens for configuration changes and rebroadcasts initial data when either
setting is updated from elsewhere.

Made-with: Cursor
… edits

When a user adds, removes, or resets subdivision breaks on a source
document, the placement mirror was already being persisted to the paired
target file but the open target webview was never told to re-render —
users had to close and reopen the target tab to see the change. This
threads the existing milestone-refresh path through to the target panel
so the change appears immediately.

The cost per source-side break edit is one extra postMessage plus a
milestone-index rebuild on the target (cached and invalidated by
updateCellData), which is negligible even on older hardware. When no
target webview is open the refresh is skipped silently and the target
picks up the change on next load via the persisted file.

Subdivision name edits remain per-side by design and don't trigger a
target refresh.

Made-with: Cursor
The milestone accordion now opens in a read-only baseline: the only
edit-related control in the header is a gear icon. Clicking the gear
reveals the title pencil, surfaces the per-subsection rename pencils
(always visible — no more hover-only reveal), and unhides the source-
only "Add break…" / "Reset" footers. Re-clicking the gear collapses the
affordances back, and reopening the dropdown always starts from the
read-only baseline so the gear-on state is never sticky across sessions.

The "Add break at cell" placeholder also drops its compact "2–N" range
hint in favor of a single illustrative number, matching the underlying
input semantics (one cell number, not a range).

Tests opt back into settings mode by default via a new `initialSettingsMode`
prop so the existing edit/rename/break test suites keep exercising those
flows without first having to click the gear; three new tests cover the
gear toggle itself.

Made-with: Cursor
…istence

- resolveSubdivisions now sub-chunks long stretches between user-defined
  breaks by cellsPerPage, so adding a break at cell 433 in a 700-cell
  milestone still produces [1-50]…[401-432],[433-482]…[683-700] rather
  than two huge pages.
- New workspace setting codex-editor-extension.maxSubdivisionLength (0 =
  off): when positive, stretches between user breaks shorter than this
  value are left intact (e.g. a 103-cell logical page survives with
  threshold=120), while longer stretches continue to be sub-chunked.
  maxSubdivisionLength is threaded through buildMilestoneIndex, getCells-
  ForMilestone, findMilestoneAndSubsectionForCell, and the config-change
  listener so all callers stay in sync.
- Source subdivision names are now the default for the paired target
  document. Names ride on the placement object and are mirrored into
  subdivisionNamesFromSource; resolveSubdivisions picks them up as a
  fallback when the target has no local override. A target translator
  can still set their own independent name.
- Naming an auto-generated chunk (on the source side) promotes it to a
  persistent placement so that changes to cellsPerPage or maxSubdivision-
  Length cannot displace or orphan the name. Clearing a name leaves the
  placement intact; the implicit first subdivision is never promoted.
- Cache-invalidation fix: updateCellData now also busts the milestone
  index when subdivisionNamesFromSource changes.
- 24 new unit / integration tests covering all of the above.

Made-with: Cursor
- New "Maximum subdivision length" toggle + numeric input in the
  Pagination & Subdivisions section. When enabled, defaults to
  cellsPerPage × 2. Wired to the codex-editor-extension.maxSubdivision-
  Length workspace setting with validation (clamped 1-5000).
- Temporarily hides the "Always show subdivision number ranges" toggle
  (the underlying state and message wiring are preserved); names are
  always shown since numeric ranges are already visible in lighter text.
- Removes the "(between 5 and 200)" parenthetical from the Cells per
  page description.
- Updates the Maximum subdivision length description to: "Pagination
  allows ranges between user added subdivisions up to this length".

Made-with: Cursor
- Replace the per-subdivision X remove button and the milestone-level
  reset button icons with Trash2 (lucide), tinted vscode-errorForeground.
- Rows that cannot be deleted (auto-chunks, target side, first
  subdivision) now render an invisible aria-hidden phantom span the same
  size as the trash-can button (28×28 px, visibility:hidden) so the
  rename pencil icons stay vertically aligned across all rows.

Made-with: Cursor
This keeps row spacing consistent with the active delete button in milestone settings mode.
Adds a touch more separation between row controls and progress dots.
Ensure the clicked milestone expands before entering edit mode, and move the rename affordance into the milestone row.
The /build CI was failing because the webview test suite couldn't load
MilestoneAccordion: the lucide-react mock explicitly listed icons and
silently broke the moment the component imported a new one (Trash2).
Even after fixing the mock, ~30 assertions still pointed at the old
single-pencil "Edit Milestone" label that no longer exists, and the new
per-row "Rename Milestone" pencils caused getByLabelText to throw on
multi-element matches.

Two coupled fixes, one outcome:

1. Standardize rename-flow aria-labels around the user-facing nouns:
   - Milestone rename: "Rename Milestone" / "Save Milestone Rename" /
     "Cancel Milestone Rename" (was "Edit/Save Milestone" + "Revert
     Changes").
   - Milestone subdivision rename: "Rename/Save/Cancel Milestone
     Subdivision Rename" (was "Rename/Save/Cancel Subsection [Name]").
   Non-rename actions (gear, add/remove break, reset) keep their
   existing names.

2. Make the test queries match the new UI:
   - vi.mock("lucide-react") now uses importOriginal so newly imported
     icons don't silently break the suite again.
   - Per-milestone rename pencils are looked up via a small helper that
     scopes within the row's data-testid="accordion-item-{idx}", so
     tests read as "Chapter 1's pencil" rather than "the first
     getByLabelText match".
   - Describe blocks renamed Edit Mode -> Milestone Rename and
     Subsection Rename -> Milestone Subdivision Rename to match the
     user-facing language.
Pulls `buildMilestoneCellPayload` and the chapter/book-name helpers out
of `migrationUtils.ts` so importers, migrations, and in-editor
structural edits can share a single source of truth for milestone-cell
shape (label format, edit-history, kind/languageId envelope, optional
initial `metadata.data`).

Pure refactor — no behavior change.
Three additive primitives the new milestone-placement edits need:

- `splitPlacementsAtAnchor` partitions a milestone's
  `MilestoneSubdivisionPlacement[]` at a boundary cell, lifting any
  pre-existing subdivision *at* the boundary onto the new milestone as
  its implicit first-subdivision name (so "Section B" becomes the
  promoted milestone's label and stops doubling as a subdivision).
- `mergePlacementsForRemovedMilestone` merges two milestones' placement
  lists when one is removed or demoted; the demote path preserves the
  boundary as a new placement carrying the demoted milestone's label.
- `CodexDocument.insertMilestoneCell` adds a top-level milestone cell
  at a chosen anchor with no `parentId`, using `buildMilestoneCellPayload`
  for shape parity with importers/migrations. Backed by a `force` flag
  on `updateCellMilestoneIndices` so structural edits that don't change
  the cell count still reflush the SQLite milestone-index FTS.

No callers yet — wired up in the next commit.
Lets translators add, remove, promote (subdivision → milestone), and
demote (milestone → subdivision break) milestones on the source side.
Mirrors immediately to the paired target file when the milestone roots
agree, preserving UUIDs and anchor cell IDs. Gated end-to-end by a new
workspace setting `enableMilestonePlacementEditing` (default off) and
surfaced in Interface Settings.

Wiring:

- `package.json`: declare the new boolean setting.
- `interfaceSettings.ts` + `InterfaceSettings/index.tsx`: surface the
  toggle in the Pagination & Subdivisions section and propagate live
  changes back to the editor webview.
- `codexCellEditorProvider.ts`: read the setting, plumb it into both
  initial-content payloads, and broadcast preference updates.
- `codexCellEditorMessagehandling.ts`: four new handlers
  (`addMilestoneAtCell`, `removeMilestone`,
  `promoteSubdivisionToMilestone`, `demoteMilestoneToSubdivision`)
  built on a pair of helpers — `commitSplitMilestoneAtAnchor` and
  `commitMergeMilestoneIntoPrevious` — that handle source validation,
  subdivision redistribution, source/target mirror with root-divergence
  guards, milestone-index reflush (`force: true`), and webview refresh.
- `MilestoneAccordion.tsx`: source-only, settings-mode-only controls
  for add/remove/promote/demote, with a two-click confirmation pattern
  on the destructive demote/remove paths.
- `types/index.d.ts`: new `EditorPostMessages` commands, the
  `enableMilestonePlacementEditing` field on
  `providerSendsInitialContentPaginated`, and a new
  `updateMilestonePlacementEditingPreference` push.
Mocha (extension host):

- `splitPlacementsAtAnchor` and `mergePlacementsForRemovedMilestone`
  cover boundary lifting (named subdivision → first-subdivision
  override on the new milestone), the `preserveBoundary` toggle that
  separates `removeMilestone` from `demoteMilestoneToSubdivision`, and
  the cell-ID partition by root order.
- The four structural-edit handlers exercise the happy path
  (partition + mirror), the source-only guard (warns + no-op on
  target), the first-milestone refusal for remove/demote, and the
  promoted-from-named-subdivision label takeover.

Vitest (webview):

- `MilestoneAccordion` placement-editing controls are scoped to the
  feature setting + `isSourceText` + settings mode. Verifies the
  `addMilestoneAtCell` form payload, the promote button's
  `subdivisionKey` payload, the two-click arming for remove/demote,
  and that the first milestone never exposes destructive actions.
Brings milestone-rename UX to parity with the subdivision rename: the
pencil now swaps the milestone label for an input on its own row
(instead of swapping the dropdown header), and the matching
save/cancel cluster takes over the row's pencil/destructive area.

State changes:

- `isEditingMilestone: boolean` becomes `editingMilestoneIdx: number | null`
  so we know WHICH row hosts the input (multiple milestones share the
  same accordion).
- `handleMilestoneExpansion` no longer chases the user's expansion to
  re-target the editor — the input is anchored to the milestone it was
  opened on, just like subdivisions.
- The header is now a static `<h2>` + persistent gear button, so the
  settings toggle stays reachable while a rename is in flight (e.g.
  the user can still close the accordion mid-edit).

The `Save Milestone Rename` / `Cancel Milestone Rename` aria-labels
move with the buttons unchanged, so per-row scoped tests keep working.
The starting-rename test is updated to reflect the inline anchor and
the gear staying put.
Demote is reversible — promote turns the resulting subdivision break
right back into a milestone — so the two-click arm/confirm step was
unnecessary friction. Drops the `demoteConfirm*` state and timer, the
arming aria-label/title swap, and the matching armed-styling on the
button. Pure remove keeps its two-click confirmation since it drops the
seam entirely.

Test now asserts a single click commits and that the "Confirm Demote
Milestone" arming label never appears.
When neither the caller nor the boundary subdivision provides a label
(typical for `addMilestoneAtCell`, or for promoting an unnamed
subdivision break), `commitSplitMilestoneAtAnchor` was falling through
to `buildMilestoneCellPayload`'s chapter-style auto-label — which
clones the parent milestone's name (e.g. two "Luke 1"s side by side).

Switch the cascade to: explicit caller override → boundary placement
name → subdivisionNames map entry → "New milestone" placeholder.

Centralises the placeholder as a `NEW_MILESTONE_DEFAULT_LABEL`
constant. Adds two Mocha cases: addMilestoneAtCell stamping the
placeholder, and promoteSubdivisionToMilestone using it when the
underlying subdivision break is unnamed.
The promote/demote/add/remove handlers were awaiting two slow steps
(saveCustomDocument + updateCellMilestoneIndices, on both source and
target) BEFORE the webview refresh, so the user saw the new milestone
structure only after a full disk write + SQLite FTS reflush per
document. With both sides involved this could be 500ms–1s of dead
time per click — visible enough that the user reported it as
"delay".

Reorder both `commitSplitMilestoneAtAnchor` and
`commitMergeMilestoneIntoPrevious` to:

1. Apply the in-memory mutations on source and target.
2. Adjust cached cursor positions on both URIs.
3. Push `sendMilestoneRefreshToWebview` to source AND target panels
   (the milestone index they read is rebuilt from in-memory state, so
   it's already correct at this point).
4. Run `saveCustomDocument` + `updateCellMilestoneIndices` in parallel
   for source AND target via `Promise.all` AFTER the UI is updated.

If a save fails the user gets a notification — the in-memory state
already reflects the change (so subsequent edits compose correctly),
matching the prior behavior on partial-save errors. Existing handler
tests stub both persistence calls and assert on document state, so
they keep passing under the new ordering.
The structural mirror already stamps the source's label on the target
when a milestone is added/promoted, but subsequent renames stayed local
to the source. Now `updateMilestoneValue` propagates the new label to
the paired target's `cell.value` whenever the rename originates on a
source file, gated by matching milestone cell IDs and root cell IDs so
diverged structures are left alone. Target-side renames remain local;
placements stay source-authoritative.

Also reorders the handler's persistence step to mirror the snappy
pattern used by the structural helpers: in-memory mutate both docs,
refresh both webviews, then save in parallel — so the rename appears
without waiting on disk I/O.
Apply the same in-memory-mutate / refresh-then-persist ordering already
used by the structural milestone helpers to the two remaining mirror
paths: `commitMilestoneSubdivisionPlacements` (subdivision break
add/remove + the promotion path of subdivision rename) and the non-
promotion path of `updateMilestoneSubdivisionName`. Both webview panels
now see the new placements / `subdivisionNamesFromSource` immediately,
and the source + target saves run in parallel after the UI has already
updated — eliminating the disk-I/O wait the user could perceive on
slower filesystems.
The CodexCellEditor stale guard rejected any
providerSendsInitialContentPaginated whose currentMilestoneIndex /
currentSubsectionIndex did not match the webview's refs. That guard is
meant to keep an in-flight initial-content message from clobbering user
navigation, but it also rejected every refresh the provider pushed after
a structural milestone edit shifted the cursor (e.g. demote moves the
viewer from milestone N to N-1). The accordion stayed frozen on the
pre-edit structure and refreshCurrentPage re-requested cells from the
now-stale ref position.

Add a force flag to providerSendsInitialContentPaginated and
refreshCurrentPage; sendMilestoneRefreshToWebview sets it on every push
since by construction it only fires after the provider has mutated the
document. The webview bypasses the stale guard, drops any in-flight
navigation request, and realigns its refs to the message's position so
the follow-up cell request hits the new range.
commitMergeMilestoneIntoPrevious mutated the source first and then
called sourceAndTargetMilestoneRootsMatch on the post-mutation source to
gate the structural mirror. After the merge, the source's previous
milestone has absorbed the removed milestone's roots, so its root id
list never matched the still-unmutated target. rootsMatchPrev always
returned false and every remove/demote silently skipped the target —
the target stayed on the pre-edit structure even when both sides shared
identical milestone ids and roots before the click.

Snapshot both the previous and removed milestones' root id lists from
the source before any mutation, then compare those snapshots against
the target's current roots. The divergence guard still fires when the
target genuinely drifted, but a freshly-paired source / target pair
now mirrors cleanly through demote and remove.

Tests cover the mirrored delete, the mirrored demote (including the
boundary placement and subdivisionNamesFromSource carry-over), and the
no-op when target milestone ids have diverged.
Merge demote/remove on the paired file now folds the translator's
subdivisionNames from both milestones onto the surviving cell, instead of
leaving overrides stranded on the soft-deleted milestone. When promoting
a break on the source, a target-only name at the seam wins for the new
milestone label so it is not replaced by a source-only placeholder.

Tests cover demote (names move to survivor) and promote (translator seam
label on the new milestone).
….name

Target mirror now preserves the seam label on the subdivision row rather
than only subdivisionNamesFromSource; update assertions accordingly.
The accordion's focus-trap useEffect bundled `accordionRef.current.focus()`
with the ESC + click-outside listener registration, and depended on the
parent-supplied `onClose` callback. ChapterNavigationHeader passed `onClose`
as an inline arrow, so every parent re-render produced a fresh reference,
churning the deps and re-stealing focus from any open inline rename input.

Subdivision renames (input lives in a plain div inside AccordionContent)
were the visible casualty; milestone renames survived because their input
lives inside an AccordionTrigger button that has its own focus management.

Split the effect into two:
- one that auto-focuses the accordion exactly once per open transition,
- one that re-attaches the ESC / click-outside listeners when `onClose` flips
  (cheap, no focus side effect).

Also stabilize `onClose` in ChapterNavigationHeader with `useCallback` so
the listener-attach effect doesn't churn unnecessarily.

Adds a regression test that re-renders the accordion with a new `onClose`
identity while the subdivision rename input is focused and asserts focus
stays on the input.
@Luke-Bilhorn Luke-Bilhorn requested a review from LeviXIII May 18, 2026 15:51
@Luke-Bilhorn Luke-Bilhorn linked an issue May 18, 2026 that may be closed by this pull request
@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

When testing, make sure to go into interface settings, and enable milestone editing. Otherwise, it is disabled.

@LeviXIII LeviXIII left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pagination & Subdivisions Feedback

  1. Remove the extra space and period from "Cells per page"
  2. Change wording as "this" isn't on the screen yet (the input only appears after enabling). Maybe you can say "...subdivisions up to the set length".
Image

Source Controls

  1. Just need some tooltips to explain what the arrow does. I thought it meant to reorder, but I see that it just moves the selection between being a break and a milestone. Maybe find another icon to suggest exchange?
  2. I would remove the ellipsis off of the buttons and add captials for Add Break and Add Milestone
Image

Webview

  1. When adding breaks, the line numbers aren't accurate. I think they are still lining up by the overall cell count set in the settings.
  2. Breaks and Milestone changes aren't reflected in the target webview (for audio files at least).
  3. After making breaks and/or milestones, if I close the dropdown and then open it again, the delete for the first selection of break/milestones is enabled (it should be disabled)
Image

@LeviXIII LeviXIII left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When adding a break, if I edit the cell in the target, it seems to save, but doesn't appear in the cell until I reload the webview. Any edits to the text after that appear as they should.

Luke-Bilhorn and others added 3 commits June 25, 2026 15:21
- Interface Settings: drop stray period on "Cells per page" and reword the
  max-subdivision-length copy ("...up to the set length").
- Milestone accordion: replace the promote/demote arrow icons with a Replace
  glyph for promote and a custom up-left bent-arrow glyph for demote, with
  clearer exchange-style tooltips; capitalize and de-ellipsize the
  "Add Break" / "Add Milestone" buttons.
- Hide the delete control on the implicit first subdivision even after the
  resolver re-labels it "custom" once a break exists (gate on startIndex > 0),
  fixing the stray trash that appeared on row 0 after reopening the dropdown.
- Fix subsection line numbering after custom breaks by deriving the offset
  from the resolved subdivision's startRootIndex instead of assuming uniform
  cellsPerPage-sized pages.
@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

For 10.2, we have equivalent behavior. We don't allow placement at the actual milestone boundary, because the user already has a subdivision there that they can rename.

@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

Todo: Milestone deletion is not available.

They can demote it and then delete the subdivision, though, if they need to.

@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author
image

@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

Checking off all of section 16 because the cell editor does work

@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

17.2 Should be deleted

@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

Section 1 was not necessary, confirmed

Move into src/test/suite/, convert to TDD, and align assertions with
INITIAL_IMPORT milestone shape plus paired UUID/orphan cases.
Show "Click again to confirm" below the full settings icon row when
remove is armed, matching reset-subdivisions styling so icons don't
shift. Keep the longer merge explanation in the tooltip only.
Constrain accordion trigger/content width and use min-w-0 flex shrinking
so milestone titles and subdivision labels ellipsize instead of wrapping
or overflowing the panel.
@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

That paratext bug we noticed earlier happens on main also. I also see that adding paratext cells after adding child cells pushes those child cell numbers up.

@Luke-Bilhorn

Copy link
Copy Markdown
Contributor Author

As of now, everything is accounted for, except testing 3-way sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Milestones Editable

2 participants