Make review feedback editable and newest-first

src/chrome-client.js

@@ -5,7 +5,7 @@
  * hostname: a separate origin that can never reach this page or its token.
  */
 import { tidy } from "./anchor-text.js";
-import { pageUrl, replacePage } from "./chrome-session.js";
+import { newestComments, pageUrl, replacePage } from "./chrome-session.js";
 import { framePolicy } from "./frame-policy.js";
 
 const $ = (id) => document.getElementById(id);
@@ -139,7 +139,7 @@ function render() {
   const page = state.page;
   if (!page) return;
 
-  const comments = page.comments || [];
+  const comments = newestComments(page.comments);
   const edits = page.edits || [];
 
   $("count").textContent = String(comments.length);
@@ -175,9 +175,21 @@ function render() {
     sep.textContent = "·";
     const when = document.createElement("span");
     when.className = "when";
-    when.textContent = ago(comment.createdAt);
+    when.textContent = ago(comment.updatedAt || comment.createdAt);
     who.append(sep, when);
 
+    if (comment.correction) {
+      const badge = document.createElement("span");
+      badge.className = "badge correction";
+      badge.textContent = "correction";
+      who.append(badge);
+    } else if (comment.updatedAt) {
+      const badge = document.createElement("span");
+      badge.className = "badge";
+      badge.textContent = "edited";
+      who.append(badge);
+    }
+
     if (state.orphans.has(comment.id)) {
       const badge = document.createElement("span");
       badge.className = "badge";
@@ -194,6 +206,15 @@ function render() {
       setActive(comment.id, true);
     });
 
+    const edit = document.createElement("button");
+    edit.type = "button";
+    edit.className = "jump edit-comment";
+    edit.textContent = "Edit";
+    edit.addEventListener("click", (event) => {
+      event.stopPropagation();
+      editComment(card, body, comment);
+    });
+
     const remove = document.createElement("button");
     remove.type = "button";
     remove.className = "remove";
@@ -207,8 +228,6 @@ function render() {
       render();
     });
 
-    head.append(who, jump, remove);
-
     const quote = document.createElement("p");
     quote.className = "quote";
     quote.textContent = tidy(comment.quote, 140);
@@ -222,6 +241,8 @@ function render() {
       editComment(card, body, comment);
     });
 
+    head.append(who, jump, edit, remove);
+
     card.append(head, quote, body);
     card.addEventListener("click", () => setActive(comment.id, false));
     list.append(card);
@@ -379,11 +400,21 @@ function editComment(card, body, comment) {
     const feedback = input.value.trim();
     if (!feedback || feedback === comment.feedback) return render();
     try {
-      state.page = (await api(`/api/page/${state.key}/comment/${comment.id}`, {
+      const result = await api(`/api/page/${state.key}/comment/${comment.id}`, {
         method: "PATCH",
         body: JSON.stringify({ feedback }),
-      })).page;
-      state.sent = false;
+      });
+      state.page = result.page;
+      if (result.delivery === "updated-pending") {
+        toast("Updated the feedback waiting for your agent");
+      } else if (result.delivery === "correction") {
+        state.sent = false;
+        toFrame({ type: "eh:remove", id: comment.id });
+        toFrame({ type: "eh:anchors", comments: state.page.comments });
+        toast("Saved as a correction — send it after the current batch is acknowledged");
+      } else {
+        state.sent = false;
+      }
     } catch (err) {
       toast(err.message);
     }

src/chrome-session.js

@@ -6,3 +6,10 @@ export function replacePage(state, page) {
   state.page = page;
   state.others = page.others || [];
 }
+
+/** New and newly revised feedback belongs where the reviewer can see it. */
+export function newestComments(comments) {
+  return [...(comments || [])].sort(
+    (a, b) => (b.updatedAt || b.createdAt || 0) - (a.updatedAt || a.createdAt || 0)
+  );
+}

src/chrome.css

@@ -254,6 +254,7 @@ body.collapsed .handle { right: 0; }
   cursor: pointer;
 }
 .jump:hover { color: var(--strong-txt); }
+.edit-comment { margin-left: 0; }
 .remove {
   width: 18px;
   height: 18px;
@@ -321,6 +322,7 @@ body.collapsed .handle { right: 0; }
   font-size: 10px;
   font-weight: 600;
 }
+.badge.correction { background: var(--danger-soft); color: var(--danger); }
 
 .empty {
   padding: 16px 14px;

src/server.js

@@ -222,6 +222,7 @@ export function createServer() {
           quote: c.quote,
           anchor: c.anchor,
           feedback: c.feedback,
+          ...(c.correction ? { correction: true, correction_of: c.correctionOf } : {}),
         })),
         edits: page.edits.map((e) => ({
           label: e.label,
@@ -257,6 +258,7 @@ export function createServer() {
 
     const hasMarkdown = pages.some((p) => p.kind === "file" && isMarkdown(p.file));
     const hasUrl = pages.some((p) => p.kind === "url");
+    const hasCorrections = pages.some((p) => p.comments.some((c) => c.correction));
     const batch = {
       status: "feedback",
       pages: pages.map(({ kind, file, url, comments, edits }) => ({ kind, file, ...(url ? { url } : {}), comments, edits })),
@@ -277,6 +279,9 @@ export function createServer() {
             "When an edit includes `staged_assets`, copy each local image into the app's appropriate asset folder, replace its " +
             "temporary preview URL in `after_html`, and preserve the image at the user's insertion point. "
           : "") +
+        (hasCorrections
+          ? "Comments marked `correction` replace their `correction_of` instruction; follow the correction and do not apply the older wording. "
+          : "") +
         "When every page is updated, run the same poll command again with --ack to clear this " +
         "batch and wait for more.",
     };
@@ -673,8 +678,37 @@ export function createServer() {
           const body = await readBody(req);
           const feedback = String(body.feedback || "").trim();
           if (!feedback) return json(res, 400, { error: "empty feedback" });
-          if (!store.updateComment(key, tail, feedback)) return json(res, 404, { error: "unknown comment" });
-          return json(res, 200, { page: pageState(key) });
+          const existing = store.page(key).comments.find((comment) => comment.id === tail);
+          if (!existing) return json(res, 404, { error: "unknown comment" });
+
+          const matchingCleanup = (record) =>
+            record.cleanup.some((item) => item.key === key && item.ids.includes(tail));
+          const wasDelivered = [...batches.values()].some((record) => record.delivered && matchingCleanup(record));
+
+          if (wasDelivered) {
+            // The agent already received the old wording, so the revision must
+            // survive that batch's ack and explicitly supersede it next time.
+            store.updateComment(key, tail, feedback, {
+              replacementId: uid("c"),
+              correctionOf: existing.feedback,
+            });
+            return json(res, 200, { delivery: "correction", page: pageState(key) });
+          }
+
+          store.updateComment(key, tail, feedback);
+          let updatedPending = false;
+          const target = store.page(key);
+          const targetName = target.kind === "url" ? target.url : target.file;
+          for (const [entryKey, record] of batches) {
+            if (record.delivered || !matchingCleanup(record)) continue;
+            const batchPage = record.batch.pages.find((page) => page.file === targetName || page.url === targetName);
+            const batchComment = batchPage?.comments.find((comment) => comment.id === tail);
+            if (!batchComment) continue;
+            batchComment.feedback = feedback;
+            store.setBatch(entryKey, record);
+            updatedPending = true;
+          }
+          return json(res, 200, { delivery: updatedPending ? "updated-pending" : "unsent", page: pageState(key) });
         }
 
         if (action === "edit" && req.method === "POST") {

src/state.js

@@ -189,13 +189,21 @@ export class Store {
     });
   }
 
-  /** Rewording feedback before it is sent. Returns null for an unknown id. */
-  updateComment(key, id, feedback) {
+  /** Reword feedback, optionally turning a delivered instruction into a correction. */
+  updateComment(key, id, feedback, { replacementId = "", correctionOf = "" } = {}) {
     let found = false;
     const page = this.update(key, (p) => {
-      const comment = p.comments.find((c) => c.id === id);
+      const index = p.comments.findIndex((c) => c.id === id);
+      const comment = p.comments[index];
       if (comment) {
-        comment.feedback = feedback;
+        const updated = {
+          ...comment,
+          ...(replacementId ? { id: replacementId } : {}),
+          feedback,
+          updatedAt: Date.now(),
+          ...(correctionOf ? { correction: true, correctionOf } : {}),
+        };
+        p.comments[index] = updated;
         found = true;
       }
     });

test/chrome-session.test.js

@@ -1,7 +1,7 @@
 import test from "node:test";
 import assert from "node:assert/strict";
 
-import { pageUrl, replacePage } from "../src/chrome-session.js";
+import { newestComments, pageUrl, replacePage } from "../src/chrome-session.js";
 
 test("page refreshes keep session context and clear stale cross-page counts", () => {
   assert.equal(pageUrl("abc123", "session with spaces"), "/api/page/abc123?session=session%20with%20spaces");
@@ -17,3 +17,14 @@ test("page refreshes keep session context and clear stale cross-page counts", ()
   assert.equal(state.page, refreshed);
   assert.deepEqual(state.others, []);
 });
+
+test("comments are newest-first and an edited comment moves to the top", () => {
+  const comments = [
+    { id: "old", createdAt: 100 },
+    { id: "new", createdAt: 300 },
+    { id: "edited", createdAt: 50, updatedAt: 400 },
+  ];
+
+  assert.deepEqual(newestComments(comments).map((comment) => comment.id), ["edited", "new", "old"]);
+  assert.deepEqual(comments.map((comment) => comment.id), ["old", "new", "edited"], "stored order is not mutated");
+});

test/feedback-safety.test.js

@@ -152,6 +152,7 @@ test("a comment can be reworded before it is sent", async (t) => {
   });
   assert.equal(reworded.status, 200);
   assert.deepEqual(j(reworded).page.comments.map((c) => c.feedback), ["Sharper thoughts"]);
+  assert.equal(typeof j(reworded).page.comments[0].updatedAt, "number");
 
   const missing = await request(port, token, { method: "PATCH", route: `/api/page/${key}/comment/c_nope`, body: { feedback: "x" } });
   assert.equal(missing.status, 404);
@@ -165,6 +166,56 @@ test("a comment can be reworded before it is sent", async (t) => {
   await ackAndAbandon(port, token, file);
 });
 
+test("rewording replaces a waiting batch and becomes a correction after delivery", async (t) => {
+  const file = path.join(tmp, "reword-after-send.html");
+  fs.writeFileSync(file, "<!DOCTYPE html><html><body><p>Draft</p></body></html>");
+  const { port, token, dispose } = await start(0);
+  t.after(() => dispose());
+
+  const opened = j(await request(port, token, { method: "POST", route: "/api/session", body: { file } }));
+  const added = j(await request(port, token, {
+    method: "POST",
+    route: `/api/page/${opened.key}/comment`,
+    body: { kind: "selection", quote: "Draft", feedback: "Delete this" },
+  }));
+
+  // Nothing is polling yet, so editing should replace the stranded batch in place.
+  await request(port, token, { method: "POST", route: `/api/page/${opened.key}/send`, body: { sessionId: opened.sessionId, note: "" } });
+  const waitingEdit = j(await request(port, token, {
+    method: "PATCH",
+    route: `/api/page/${opened.key}/comment/${added.comment.id}`,
+    body: { feedback: "Shorten this" },
+  }));
+  assert.equal(waitingEdit.delivery, "updated-pending");
+
+  const delivered = j(await request(port, token, { route: `/api/poll?target=${encodeURIComponent(file)}` }));
+  assert.equal(delivered.pages[0].comments[0].feedback, "Shorten this", "the agent never sees the superseded wording");
+
+  // Once delivered, a further revision must survive ack as an explicit correction.
+  const correction = j(await request(port, token, {
+    method: "PATCH",
+    route: `/api/page/${opened.key}/comment/${added.comment.id}`,
+    body: { feedback: "Keep it, but add an example" },
+  }));
+  assert.equal(correction.delivery, "correction");
+  assert.equal(correction.page.comments[0].correction, true);
+  assert.equal(correction.page.comments[0].correctionOf, "Shorten this");
+  assert.notEqual(correction.page.comments[0].id, added.comment.id, "the delivered id is retired so ack cannot clear the correction");
+
+  await ackAndAbandon(port, token, file);
+  const afterAck = j(await request(port, token, { route: `/api/page/${opened.key}` }));
+  assert.equal(afterAck.comments.length, 1);
+  assert.equal(afterAck.comments[0].feedback, "Keep it, but add an example");
+
+  await request(port, token, { method: "POST", route: `/api/page/${opened.key}/send`, body: { sessionId: opened.sessionId, note: "" } });
+  const correctedBatch = j(await request(port, token, { route: `/api/poll?target=${encodeURIComponent(file)}` }));
+  const shipped = correctedBatch.pages[0].comments[0];
+  assert.equal(shipped.correction, true);
+  assert.equal(shipped.correction_of, "Shorten this");
+  assert.match(correctedBatch.next_step, /replace their `correction_of` instruction/);
+  await ackAndAbandon(port, token, file);
+});
+
 test("a save based on a stale version of the file is refused", async (t) => {
   const file = path.join(tmp, "save.html");
   const v1 = "<!DOCTYPE html>\n<html><head></head><body><p>One</p></body></html>\n";