Drafts rebase onto the live settings; the review speaks in words

A draft started before someone else's Apply used to carry that older config and would have put the newer changes back. Now every read of the draft replays only the draft's own changes onto the live settings (base config from history), so the review lists exactly what you changed. Values read in words (windows as days and hours), long ones fold open with was/now, commands are no longer red, and warnings the live config already had are not repeated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-10-14 19:44:51 -04:00
parent edfd218ac9
commit 0eee778a92
6 changed files with 181 additions and 22 deletions
@@ -176,22 +176,105 @@ def words_for(path, before, after, cfg_before, cfg_after):
    elif where.endswith(field):
        where = where[:-len(field)] + field.replace("_", " ")

-    def val(v):
+    val = plain_value
+    secret = any(f["secret"] for f in schema.F.values() if f["path"].endswith(field))
+    if secret:
+        return {"path": path, "where": where, "text": "%s changed" % where, "command": False}
+    command = field in ("run", "cmd", "match") or path.endswith("pref.want")
+    b, a = val(before), val(after)
+    long_ = command or len(b) + len(a) > 90
+    if before is not None and after is not None:
+        text = "%s changed" % where if long_ else "%s: %s → %s" % (where, b, a)
+    elif before is None:
+        text = "%s added" % where if long_ else "%s added: %s" % (where, a)
+    else:
+        text = "%s removed" % where if long_ else "%s removed (was %s)" % (where, b)
+    out = {"path": path, "where": where, "text": text, "help": help_, "command": command}
+    if long_:
+        out["before"], out["after"] = (b if before is not None else None), (a if after is not None else None)
+    return out
+
+
+def hhmm12(t):
+    m = re.match(r"^(\d{1,2}):(\d{2})$", str(t or ""))
+    if not m:
+        return str(t)
+    h, mm = int(m.group(1)), int(m.group(2))
+    if h == 24 and mm == 0:
+        return "midnight"
+    return "%d%s %s" % (h % 12 or 12, (":%02d" % mm) if mm else "", "am" if h < 12 else "pm")
+
+
+def plain_value(v):
+    """A value in words: windows as days and hours, lists joined, records as 'field value' pairs."""
    if v is None:
        return "—"
    if isinstance(v, bool):
        return "yes" if v else "no"
-        if isinstance(v, (dict, list)):
-            s = json.dumps(v, ensure_ascii=False)
-            return s if len(s) < 80 else s[:77] + "..."
+    if isinstance(v, list):
+        if v and all(isinstance(w, dict) and "days" in w and "start" in w for w in v):
+            return "; ".join("%s %s – %s" % (", ".join(d[:1].upper() + d[1:] for d in w.get("days", [])), hhmm12(w.get("start")), hhmm12(w.get("end"))) for w in v) or "never"
+        if all(isinstance(x, (str, int, float)) for x in v):
+            return ", ".join(str(x) for x in v) or "none"
+        return "; ".join(plain_value(x) for x in v)
+    if isinstance(v, dict):
+        return ", ".join("%s %s" % (k.replace("_", " "), plain_value(x)) for k, x in v.items()) or "nothing"
    return str(v)
-    secret = any(f["secret"] for f in schema.F.values() if f["path"].endswith(field))
-    if secret:
-        return {"path": path, "where": where, "text": "%s changed" % where, "command": False}
-    text = "%s: %s → %s" % (where, val(before), val(after)) if before is not None and after is not None else \
-        ("%s added: %s" % (where, val(after)) if before is None else "%s removed (was %s)" % (where, val(before)))
-    command = field in ("run", "cmd", "match") or path.endswith("pref.want")
-    return {"path": path, "where": where, "text": text, "help": help_, "command": command}
+
+
+def set_path(cfg, path, value):
+    """Put a value at a changed_paths() path (tiles[hd2].checks.0.optional, hosts.prop.ssh ...); None removes it."""
+    tokens = re.findall(r"\[([^\]]+)\]|([^.\[\]]+)", path)
+    node = cfg
+    for i, (idx, key) in enumerate(tokens):
+        last = i == len(tokens) - 1
+        if idx:                                   # a record in an id list
+            rec = next((x for x in node if isinstance(x, dict) and x.get("id") == idx), None)
+            if last:
+                if value is None:
+                    if rec is not None:
+                        node.remove(rec)
+                elif rec is None:
+                    node.append(value)
+                else:
+                    rec.clear(); rec.update(value)
+                return
+            if rec is None:
+                rec = {"id": idx}; node.append(rec)
+            node = rec
+        elif isinstance(node, list):
+            k = int(key)
+            if last:
+                if value is None:
+                    if k < len(node):
+                        node.pop(k)
+                elif k < len(node):
+                    node[k] = value
+                else:
+                    node.append(value)
+                return
+            while len(node) <= k:
+                node.append({})
+            node = node[k]
+        else:
+            if last:
+                if value is None:
+                    node.pop(key, None)
+                else:
+                    node[key] = value
+                return
+            if key not in node or node[key] is None:
+                node[key] = [] if (i + 1 < len(tokens) and (tokens[i + 1][0] or tokens[i + 1][1].isdigit())) else {}
+            node = node[key]
+
+
+def rebase(draft, live_cfg, base_cfg):
+    """A draft started on an older config: its own changes, replayed on the live config."""
+    merged = json.loads(json.dumps(live_cfg))
+    changes = changed_paths(base_cfg, draft["config"])
+    for path, _, after in changes:
+        set_path(merged, path, after)
+    return merged, changes


 def what_apply_does(cfg_before, cfg_after, hashes_before):
@@ -645,7 +645,7 @@ class Handler(http.server.BaseHTTPRequestHandler):
                changes = [applying.words_for(p, a, b, cfg, live_cfg) for p, a, b in applying.changed_paths(cfg, live_cfg)]
                return self.send(200, {"config": schema.mask_secrets(cfg), "changes_to_now": changes})
            if path == "/api/draft":
-                d = site.draft.read()
+                d = self.draft_now()
                return self.send(200, {"draft": self.masked_draft(d) if d else None, "live_sha": site.cfg_sha,
                                       "stale": bool(d and d.get("base_sha") != site.cfg_sha),
                                       "atem_pending": os.path.exists(os.path.join(site.data_dir, "draft-atem.xml"))})
@@ -667,7 +667,7 @@ class Handler(http.server.BaseHTTPRequestHandler):
                snap = site.read_json("snapshot.json", {}) or {}
                return self.send(200, {"local": snap.get("midi"), "remote": snap.get("remote_midi")})
            if path == "/api/site/names":
-                return self.send(200, self.names_check((site.draft.read() or {}).get("config") or site.load()))
+                return self.send(200, self.names_check((self.draft_now() or {}).get("config") or site.load()))
            if path == "/api/outlets":
                d = outlets.load()
                t = d.get("tuya") or {}
@@ -795,7 +795,7 @@ class Handler(http.server.BaseHTTPRequestHandler):
                self.discard_atem()
                return self.send(200, {"ok": True})
            if path in ("/api/build", "/api/build-check"):
-                cfg = (site.draft.read() or {}).get("config") or site.load()
+                cfg = (self.draft_now() or {}).get("config") or site.load()
                try:
                    if path == "/api/build":
                        fix = builders.build(cfg, body.get("fix") or {})
@@ -804,10 +804,10 @@ class Handler(http.server.BaseHTTPRequestHandler):
                except ValueError as e:
                    return self.send(200, {"ok": False, "error": str(e)})
            if path == "/api/validate":
-                cfg = schema.unmask(body.get("config") or (site.draft.read() or {}).get("config") or site.load(), site.load())
+                cfg = schema.unmask(body.get("config") or (self.draft_now() or {}).get("config") or site.load(), site.load())
                return self.send(200, self.check(cfg))
            if path == "/api/apply":
-                draft = site.draft.read()
+                draft = self.draft_now()
                if not draft:
                    return self.send(400, {"error": "there is no draft to apply"})
                if draft.get("base_sha") != site.cfg_sha:
@@ -1071,6 +1071,29 @@ class Handler(http.server.BaseHTTPRequestHandler):
            raise ValueError("no keys in it")
        return cfg

+    def draft_now(self):
+        """The draft, rebased onto the live settings when they moved on since it was started."""
+        site = self.site
+        d = site.draft.read()
+        if not d or d.get("base_sha") == site.cfg_sha:
+            return d
+        base = None
+        for e in applying.history_entries(site.data_dir):
+            meta = site.read_json(os.path.join("history", e["id"] + ".meta.json"), {}) or {}
+            if meta.get("sha_before") == d.get("base_sha"):
+                base = applying.history_config(site.data_dir, e["id"])
+                break
+        if base is None:
+            return d                                        # stale: the page says so
+        merged, changes = applying.rebase(d, site.load(), base)
+        nd, conflict = site.draft.update(merged, d.get("by", "?"), site.cfg_sha, expect=d.get("version"))
+        if nd is None:
+            return conflict
+        nd["started"], nd["started_by"] = d.get("started", nd["started"]), d.get("started_by", nd["started_by"])
+        sp.write_json(site.draft.path, nd)
+        log.info("draft by %s rebased onto the live settings (%d of its own changes kept)", d.get("by"), len(changes))
+        return nd
+
    def masked_draft(self, draft):
        return dict(draft, config=schema.mask_secrets(draft["config"]))

@@ -1083,6 +1106,8 @@ class Handler(http.server.BaseHTTPRequestHandler):
        except Exception:
            conns = None
        errors, warnings = schema.validate(cfg, {"connections": conns} if conns else None)
+        already = set(schema.validate(live_cfg)[1])
+        warnings = [w for w in warnings if w not in already]                 # only what this change adds
        tmp = tempfile.NamedTemporaryFile("wb", suffix=".json", delete=False)
        tmp.write(applying.config_text(cfg)); tmp.close()
        try:
@@ -676,11 +676,13 @@
      return;
    }
    const r = await post("/api/validate", {});
-    const changes = r.changes.map((c) => `<li class="${c.command ? "command" : ""}">${esc(c.text)}${c.command ? ' <span class="pill">command</span>' : ""}</li>`).join("") + (r.atem_pending ? "<li>A new saved switcher file replaces the current one.</li>" : "");
+    const changes = r.changes.map((c) => c.before !== undefined || c.after !== undefined
+      ? `<li><details><summary>${esc(c.text)}${c.command ? ' <span class="pill" data-hint="A command the watchdog will run; it never runs before Apply">command</span>' : ""}</summary>${c.before != null ? `<div class="small"><span class="muted">was</span> <span class="${c.command ? "mono" : ""}">${esc(c.before)}</span></div>` : ""}${c.after != null ? `<div class="small"><span class="muted">now</span> <span class="${c.command ? "mono" : ""}">${esc(c.after)}</span></div>` : ""}</details></li>`
+      : `<li>${esc(c.text)}</li>`).join("") + (r.atem_pending ? "<li>A new saved switcher file replaces the current one.</li>" : "");
    const errors = r.errors.map((e) => `<li>${esc(e)}</li>`).join("");
    const warnings = r.warnings.map((w) => `<li>${esc(w)}</li>`).join("");
    const live = r.live && r.live.live ? `<div class="notice">Live now: ${esc(r.live.reason)}. Apply waits; your draft is saved. ${editing() ? '<button id="emergency" class="danger small">Apply anyway</button>' : ""}</div>` : "";
-    main.innerHTML = `<h1>Apply${hint("Your changes in words, and what will happen on the deck. Nothing is live until you press APPLY.")}</h1><p class="muted small">Started by ${esc(state.draft.started_by || state.draft.by || "?")}${state.draft.started ? " at " + esc(when(state.draft.started)) : ""}. ${state.draftStale ? "<b>The live settings changed since: look again before applying.</b>" : ""}</p>
+    main.innerHTML = `<h1>Apply${hint("Your changes in words, and what will happen on the deck. Nothing is live until you press APPLY.")}</h1>${state.draftStale ? "<p class='notice'>The live settings changed since this draft was started and could not be merged: look through the list before applying.</p>" : ""}
      <div class="card">${changes ? `<ul class="changes">${changes}</ul>` : "<p class='muted'>No change yet.</p>"}
        <p><b>Then:</b> ${esc(r.feel)}</p>
        ${errors ? `<div class="notice problem"><b>Not possible yet:</b><ul>${errors}</ul></div>` : ""}${warnings ? `<div class="notice grey"><ul>${warnings}</ul></div>` : ""}${live}
@@ -117,7 +117,7 @@ label.btn, a.btn { display: inline-block; cursor: pointer; border: 1px solid var
 .notice { padding: 8px 12px; border-radius: 8px; background: var(--accent-soft); margin: 8px 0; } .notice.grey { background: var(--bg); border: 1px solid var(--line); color: var(--muted); }
 .notice.problem { background: #fdecea; color: #7a1d16; border: 1px solid var(--problem); }
 ol.checks li { margin: 8px 0; } .checkbox { padding: 6px 10px; margin: 4px 0; border-left: 3px solid var(--line); }
-ul.changes li.command { font-family: var(--mono); font-size: .9em; color: var(--problem); }
+ul.changes { padding-left: 18px; } ul.changes li { margin: 4px 0; } ul.changes summary { cursor: pointer; color: var(--fg); } ul.changes .mono { overflow-wrap: anywhere; }
 ol.steps { padding-left: 18px; } ol.steps li { margin: 3px 0; }
 details.script textarea { width: 100%; font-size: .8rem; }
@media (max-width: 760px) {
@@ -88,9 +88,13 @@ class TestDraftAndPlan(unittest.TestCase):
        hd2["checks"][0]["optional"] = False
        after["site"]["editing_code"] = "new words"
        after["hosts"]["lighting"]["notes"] = "keep it Public"
-        texts = [ap.words_for(p, a, b, self.cfg, after)["text"] for p, a, b in ap.changed_paths(self.cfg, after)]
+        words = [ap.words_for(p, a, b, self.cfg, after) for p, a, b in ap.changed_paths(self.cfg, after)]
+        texts = [w["text"] for w in words]
        self.assertIn("key HD2 › check 1 › optional: yes → no", texts)
-        self.assertIn("computer Lighting PC › notes: Keep the network profile on Public: rtpMIDI's Bonjour rules only allow Public. → keep it Public", texts)
+        self.assertIn("computer Lighting PC › notes changed", texts)                    # long values fold open
+        note = next(w for w in words if w["text"] == "computer Lighting PC › notes changed")
+        self.assertEqual((note["before"][:8], note["after"]), ("Keep the", "keep it Public"))
+        self.assertEqual(ap.plain_value([{"days": ["sun"], "start": "06:30", "end": "13:30"}, {"days": ["wed"], "start": "16:00", "end": "21:00"}]), "Sun 6:30 am – 1:30 pm; Wed 4 pm – 9 pm")
        self.assertTrue(any(t == "site › editing code changed" for t in texts), texts)   # a secret: never its value
        more = json.loads(json.dumps(self.cfg))
        midi = next(t for t in more["tiles"] if t["id"] == "midi")
@@ -100,6 +104,28 @@ class TestDraftAndPlan(unittest.TestCase):
        self.assertTrue(texts[0].startswith("key MIDI › check %d added: " % (len(midi["checks"]))), texts)


+class TestRebase(unittest.TestCase):
+    def test_set_path_and_rebase(self):
+        base = json.loads(json.dumps(CONFIG))
+        live = json.loads(json.dumps(base)); live["debounce_seconds"] = 7                            # moved on since the draft started
+        live["fixes"]["vista"]["label"] = "start Vista (new words)"
+        draft = {"config": json.loads(json.dumps(base))}
+        draft["config"]["auto_fix"]["windows"].append({"days": ["wed"], "start": "16:00", "end": "21:00"})   # the draft's own change
+        next(t for t in draft["config"]["tiles"] if t["id"] == "hd2")["checks"][0]["optional"] = False
+        draft["config"]["hosts"]["lighting"]["notes"] = "mine"
+        merged, changes = ap.rebase(draft, live, base)
+        self.assertEqual(merged["debounce_seconds"], 7)                                          # the live change survives
+        self.assertEqual(merged["fixes"]["vista"]["label"], "start Vista (new words)")
+        self.assertEqual(len(merged["auto_fix"]["windows"]), 2)                                  # and the draft's own
+        self.assertFalse(next(t for t in merged["tiles"] if t["id"] == "hd2")["checks"][0]["optional"])
+        self.assertEqual(merged["hosts"]["lighting"]["notes"], "mine")
+        self.assertEqual(len(changes), 3)
+        cfg = json.loads(json.dumps(base))
+        ap.set_path(cfg, "tiles[hd2]", None); self.assertFalse(any(t["id"] == "hd2" for t in cfg["tiles"]))
+        ap.set_path(cfg, "tiles[hd9]", {"id": "hd9", "label": "HD9"}); self.assertTrue(any(t["id"] == "hd9" for t in cfg["tiles"]))
+        ap.set_path(cfg, "hosts.newpc.ssh", "a@b"); self.assertEqual(cfg["hosts"]["newpc"]["ssh"], "a@b")
+
+
 class TestPipeline(unittest.TestCase):
    def setUp(self):
        self.data = tempfile.mkdtemp(prefix="sp-pipe-")
@@ -268,6 +268,29 @@ class TestServer(unittest.TestCase):
        self.assertNotIn("HyperDeck_1", gone[0]["candidates"])                 # used by HD1
        self.put("/api/draft", {}, method="DELETE")

+    def test_a_draft_is_rebased_when_the_live_settings_move_on(self):
+        from test_apply import FakeLaunchctl
+        self.site.attempts.clear()
+        fake = FakeLaunchctl(self.data); self.site.applier.run, self.site.applier.sleep = fake, lambda s: None
+        live = self.site.load()
+        mine = json.loads(json.dumps(live)); mine["hosts"]["lighting"]["notes"] = "sam's note"
+        self.put("/api/draft", {"config": mine}, who="sam")                                   # sam's draft
+        d = self.site.draft.read()
+        self.site.draft.clear()
+        other = json.loads(json.dumps(live)); other["debounce_seconds"] = (live["debounce_seconds"] or 3) + 1   # chris applies something else
+        self.put("/api/draft", {"config": other}, who="chris")
+        self.put("/api/apply", {"note": "chris", "emergency": {"word": "LIVE", "reason": "test"}}, method="POST", who="chris")
+        self.site.applier.thread.join(30)
+        sp.write_json(self.site.draft.path, d)                                                  # sam's draft is back, based on the old settings
+        st, raw = get(self.port, "/api/draft")
+        r = json.loads(raw)
+        self.assertFalse(r["stale"])
+        self.assertEqual(r["draft"]["config"]["hosts"]["lighting"]["notes"], "sam's note")      # sam's own change
+        self.assertEqual(r["draft"]["config"]["debounce_seconds"], other["debounce_seconds"])   # chris's change kept
+        st, v = self.put("/api/validate", {})
+        self.assertEqual(len(v["changes"]), 1); self.assertTrue(v["changes"][0]["text"].startswith("computer Lighting PC › notes"))
+        self.put("/api/draft", {}, method="DELETE")
+
    def test_probe_is_refused_while_a_fix_runs(self):
        json.dump({"fix-x": {"key": "hd2"}}, open(os.path.join(self.data, "fixes-running.json"), "w"))
        try: