1 files changed, 97 insertions, 31 deletions
diff --git a/docs/issue-sort-order-spec.org b/docs/issue-sort-order-spec.org
index f49e869..478b074 100644
--- a/docs/issue-sort-order-spec.org
+++ b/docs/issue-sort-order-spec.org
@@ -5,61 +5,127 @@
 
 * Status
 
-*DRAFT — design proposal; nothing in =pearl.el= has changed.* Open questions for Craig at the end.
+*Reviews incorporated through round 2; rubric =Ready* (Craig, 2026-05-25).* The first draft's "reparse the issue subtrees and rewrite" re-sort would have discarded unsaved edits; this revision makes client-side sort *move whole subtrees by =LINEAR-ID= byte-for-byte*, makes header persistence atomic with the reorder, and pins down Custom View behavior. The three caveats (Custom View refuse-server-sort in v1, toggle default =updated desc=, title-sort uses the visible heading) are *adopted as final v1 decisions*. Modified recommendations are in Review Dispositions.
 
 Companion to [[file:issue-query-spec.org][issue-query-spec.org]], which defines the =:sort= / =:order= the saved-query layer already supports. This doc covers changing the order of the *current* view interactively, without hand-editing a saved query.
 
 * Problem
 
-v1 supports =:sort= (=updated= / =priority= / =title=) and =:order= (=asc= / =desc=) on saved queries. But to change how the active file is ordered, the user has to edit =pearl-saved-queries= (or the source plist) by hand and re-run. There's no "sort this view by priority, descending" command. For a view you're actually looking at, that's the natural thing to want.
+v1 supports =:sort= (=updated= / =priority= / =title=) and =:order= (=asc= / =desc=) on saved queries. But to change how the active file is ordered, the user has to edit =pearl-saved-queries= (or the source plist) by hand and re-run. There's no "sort this view by priority, descending" command for a view you're actually looking at.
 
 * Current state
 
-- =pearl--sort-issues= (query spec) applies =:sort= / =:order= client-side at the render boundary, so a refresh always lays headings out the same way.
-- =:sort= = =priority= / =title= are client-side; =created= / =updated= map to the server =orderBy= (the only fields Linear's API orders on). The query spec documents this split.
-- The active file's =#+LINEAR-SOURCE:= header records the source plist, including any =:sort= / =:order=, so =refresh-current-view= reproduces the ordering.
+- =pearl--sort-issues= applies =:sort= / =:order= client-side at the render boundary, so a refresh always lays headings out the same way.
+- =:sort= = =priority= / =title= are client-side; =created= / =updated= map to the server =orderBy= (the only fields Linear's API orders on). The saved-query layer stores =:sort= / =:order= as *symbols*.
+- The active file's =#+LINEAR-SOURCE:= header records the source plist; =pearl--update-source-header= currently rewrites the count/timestamp fields but intentionally leaves the rest untouched, so there's no header-replace helper yet.
+- The merge-refresh path deliberately protects dirty subtrees (never overwrites a subtree with unpushed edits) — the same data-loss concern applies to any in-place reorder.
+- *Custom Views:* the =customView(id){ issues }= query carries *no* =orderBy= argument, and =pearl--query-view-async= has no order parameter. A view's order is whatever the server returns for that view.
 
 * Proposed design
 
-** The command
+** The commands
 
-=pearl-set-sort= (interactive), run in the active file:
+=pearl-set-sort= (interactive, in the active file): =completing-read= the sort key (=updated= / =created= / =priority= / =title=), then the order (=asc= / =desc=); update the recorded source; apply the new order. =pearl-toggle-sort-order= flips =asc=/=desc= on the current sort and re-applies. (Transient placement deferred — a plain =M-x= is enough for v1; the menu keys are unsettled, tracked under the transient-review task.)
 
-1. =completing-read= the sort key: =updated=, =created=, =priority=, =title=.
-2. =completing-read= (or a toggle) the order: =asc= / =desc=.
-3. Update the =:sort= / =:order= in the active file's recorded =#+LINEAR-SOURCE:=.
-4. Re-order the view (see below).
+** Completion and the symbol contract (MP4)
 
-A =pearl-toggle-sort-order= convenience command just flips =asc=/=desc= on the current sort and re-orders. Both go on the transient menu (a small "Sort" group, or under View).
+Completion *displays* strings but the source stores *symbols*: =updated= / =created= / =priority= / =title= and =asc= / =desc=. Interactive input is coerced string -> symbol before it touches the source. An unknown =:sort= / =:order= already in a source produces a =user-error= rather than silently mis-sorting, so the header stays compatible with existing =:sort priority :order asc= data.
 
-** Re-order in place vs refetch
+** Sort-key sources — sort what you're looking at (MP1)
 
-The split matters for whether a sort change needs the network:
+Client-side keys read the *current local buffer state*, not a refetch:
 
-- *Client-side sorts* (=priority=, =title=): the issues are already in the buffer. Re-sort in place — reparse the issue subtrees, reorder them, rewrite. No refetch. Fast, works offline.
-- *Server-side sorts* (=created=, =updated=): the ordering comes from the server =orderBy=, and the fetch may have been truncated at the page cap, so the correct order needs a refetch with the new =orderBy=. Re-run the source (the =refresh-current-view= path) with the updated sort.
+- =priority= from the heading's priority cookie / =LINEAR-PRIORITY= drawer as currently displayed;
+- =title= from the current heading title, stripped of TODO keyword / priority cookie / tags / identifier the same way title sync strips it (so an unsaved local title edit sorts by what you see).
 
-So =set-sort= decides: client-side key → re-sort the buffer; server-side key → refetch. The command reports which it did.
+** Re-order in place: move whole subtrees, never reconstruct (HP1)
 
-** Persistence
+Client-side sort (=priority=, =title=) *moves existing issue subtrees*; it never re-renders them from parsed data (that path, via =pearl--format-issue-as-org-entry=, would discard unsaved description/comment edits and normalize user text). The command:
 
-The change updates the active file's =#+LINEAR-SOURCE:= so a later =refresh-current-view= keeps the new order. Whether it also writes back to a named saved query in =pearl-saved-queries= is open question 3 — my lean is no by default (the active file is the scratch view; saved queries are the durable definitions), with an explicit "save this ordering to the query" as a separate step.
+1. identifies the top-level issue subtrees (each with a =LINEAR-ID=) under the single parent heading;
+2. computes each one's sort key from its current org data (above);
+3. reorders the subtree *regions* and rewrites them *byte-for-byte* — descriptions, comments, provenance drawers, and any local edits survive unchanged;
+4. leaves a non-issue / malformed subtree (no =LINEAR-ID=) in a defined position (sorted last, stable) rather than dropping it.
+
+This works on a dirty buffer precisely because it preserves text. A test edits a description and a comment locally, sorts, and asserts the exact edited text is intact afterward.
+
+** Server-side sort and Custom Views (HP2)
+
+Server-side keys (=created=, =updated=) come from the server =orderBy=, and the fetch may have been truncated at the page cap, so the correct order needs a refetch with the new =orderBy=:
+
+- *Filter / saved-query / my-open sources:* re-run the source through the refresh path with the updated =orderBy= (=pearl--query-issues-async= already accepts =order-by=).
+- *Custom View sources:* =customView.issues= has no verified =orderBy= in the API or the code, so v1 *refuses* a =created=/=updated= sort on a Custom View with a clear message ("Linear does not support server-side ordering for Custom Views; sort by priority or title to reorder the fetched issues"). Client-side keys (=priority=/=title=) still work on a view — they reorder the fetched (possibly truncated) page in place, the same subtree-move as any other source. (Verifying and adding =customView.issues(orderBy:)= is a vNext research item.)
+
+** Atomic header persistence (HP3)
+
+=#+LINEAR-SOURCE= is updated *only after* the reorder actually succeeds, so the header never claims an order the buffer doesn't show (which a later =refresh-current-view= would otherwise reproduce):
+
+- client-side: write the header only after the subtree move completes;
+- server-side: pass the updated source into the fetch; write the header only as part of a successful render/merge;
+- on a failed refetch or a dirty-buffer refusal: leave the old header unchanged and message that the sort was not applied.
+
+This needs a small helper to replace the =:sort=/=:order= in the =#+LINEAR-SOURCE= line (=pearl--source-with-sort= builds the updated plist; a header-replace helper writes it), distinct from today's count/timestamp-only =pearl--update-source-header=.
+
+** Toggle with no current sort (MP2)
+
+=pearl-toggle-sort-order= on a source with no =:sort=/=:order= defaults to =updated desc=, then toggles to =updated asc= and back. (Confirm the default.)
+
+** Outcome messages (UX)
+
+The command names exactly what it did: "Sorted current buffer by priority ascending" (client-side), "Refetched My open issues ordered by updated descending" (server-side), "Could not sort: this file has no LINEAR-SOURCE header", "Could not sort a Custom View by created; Linear has no server-side ordering for views".
 
 * Proposed v1 decisions (this feature)
 
-1. =pearl-set-sort= + =pearl-toggle-sort-order=, both on the transient menu.
-2. Client-side keys re-sort the buffer in place; server-side keys refetch.
-3. The change updates the active file's recorded source so refresh preserves it.
-4. Completion is over the known keys/orders, never free text.
+1. =pearl-set-sort= + =pearl-toggle-sort-order= (transient placement deferred).
+2. Completion displays strings, the source stores symbols; an unknown source value is a =user-error= (MP4).
+3. Client-side sort moves whole issue subtrees by =LINEAR-ID=, byte-for-byte, preserving local edits — never reconstructs from parsed data (HP1).
+4. Client-side keys read current buffer state (priority cookie/drawer, stripped heading title) (MP1).
+5. Server-side keys refetch with the new =orderBy= for filter/saved-query/my-open sources; Custom Views *refuse* server-side sort in v1 but allow client-side reorder of the fetched page (HP2).
+6. =#+LINEAR-SOURCE= is written only after a successful reorder/refetch; failure leaves it unchanged (HP3).
+7. =toggle-sort-order= with no sort defaults to =updated desc= (MP2).
+8. Active-header only — no write-back to =pearl-saved-queries= in v1 (MP3).
 
-* vNext / out of scope
+* Resolved decisions
+
+The three caveats are final:
+
+1. *Custom View server sort* (decision 5): refuse =created=/=updated= on a view in v1; client-side keys still reorder the fetched page. Verifying =customView.issues(orderBy:)= is vNext.
+2. *Toggle default* (decision 7): =updated desc= when no sort is set.
+3. Client-side =title= sort uses the visible heading title, including an unsaved local edit ("sort what I see").
+
+* Files touched
+
+- =pearl.el=: =pearl-set-sort= + =pearl-toggle-sort-order=; pure helpers =pearl--source-with-sort= (updated source plist) and a sort/order symbol validator/coercer; a =#+LINEAR-SOURCE= replace helper; a client-side subtree-region-move helper (keyed by =LINEAR-ID=, computing keys from buffer state); the server-side re-run wired through the refresh path with the Custom View refusal.
+- =docs/=: this spec.
+- =README.org=: a short "sorting the current view" note.
+
+* Test plan
 
-- Multi-key sort (e.g. priority then updated).
-- Per-heading manual reordering that sticks across refresh.
-- Exposing the full Linear =orderBy= surface if the API later un-gates the =[INTERNAL]= per-field sort.
+- =pearl--source-with-sort= updates / normalizes =:sort= and =:order= (symbols); an unknown value errors.
+- Missing =#+LINEAR-SOURCE= refuses with the named message.
+- Client-side priority/title sort moves whole subtrees and *preserves unsaved description and comment edits* (assert exact edited text after sorting).
+- Client-side sort updates =#+LINEAR-SOURCE= only after the move succeeds.
+- Server-side filter source passes the right =orderBy= + updated source to the refresh/render path.
+- Server-side fetch failure leaves the source header unchanged and messages "not applied".
+- Custom View + =created=/=updated= refuses with the view message; Custom View + =priority=/=title= reorders the fetched page in place.
+- =toggle-sort-order= with no sort/order applies the =updated desc= default.
+- Active-header sort change does not persist into =pearl-saved-queries=.
+- Malformed / non-issue subtree sorts last, stable, not dropped.
 
-* Open questions for Craig
+* Review Dispositions
+
+*Round 1 (Codex, 2026-05-25).* Rubric =Not ready=. Accepted as written: HP1 (move whole subtrees byte-for-byte rather than reparse/rewrite — the core safety fix), HP3 (atomic header persistence with rollback on failure), MP1 (client-side keys read current buffer state), MP2 (toggle default =updated desc=), MP3 (active-header only, no saved-query write-back in v1), MP4 (string display / symbol storage, unknown -> user-error), and the UX outcome messages, architecture helper shape, and expanded test plan. One was modified:
+
+- *HP2 (Custom View server sort) — modified to a decision.* The reviewer offered "verify and implement =customView.issues(orderBy:)=" or "refuse server-side sort on views". Chose refuse-in-v1: =customView.issues= has no verified =orderBy= in the API or the code, and a view already carries its own server-defined order, so adding an unverified ordering argument is out of proportion to a v1 sort command. Client-side keys still reorder the fetched page on a view. Verifying =customView.issues(orderBy:)= is recorded as a vNext research item.
+
+Everything else accepted as written.
+
+*Round 2 (Codex, 2026-05-25).* Rubric =Ready with caveats=; no new findings — MP1-MP3 were each "confirm decision X" (Custom View refuse-server-sort, toggle default =updated desc=, title-sort-uses-visible-heading), plus a test nudge (a malformed non-issue subtree fixture, already in the test plan). Resolved by adopting all three as final v1 decisions (latitude granted by Craig). Rubric -> =Ready=.
+
+* vNext / out of scope
 
-1. *Command vs transient-only*: a plain =M-x pearl-set-sort= with two completing-reads, or a dedicated transient sub-menu with one-key sort toggles (=p= priority, =u= updated, =t= title, =o= flip order)? The transient reads faster for a frequent action.
-2. *In-place re-sort fidelity*: re-sorting client-side means reparsing and rewriting issue subtrees in the buffer. Acceptable, or prefer always-refetch for simplicity even when a client-side sort wouldn't need it?
-3. *Write-back*: should changing the sort offer to persist it to the originating saved query, or only ever update the active file's header?
+- Verify + implement =customView.issues(orderBy:)= for server-side sorting of Custom Views.
+- Saved-query write-back for sort/order (a separate "save this ordering to the query" step).
+- A dedicated sort transient with one-key sort choices.
+- Multi-key sort (priority then updated).
+- Manual per-heading reordering that survives refresh.