Skip to content

Commit 07d42b3

Browse files
paddymulclaude
andcommitted
fix(#89): address review findings in deferred follow-ups
promote_diff checkpoint_step: make catalog_promote_diff checkpoint-exempt (it self-checkpoints, exactly like catalog_recalc) and stamp the recalc sub-report with that single step. Removes the redundant dispatch-decorator commit (the pre-existing empty trailing commit) so the promote + cascade are genuinely ONE revision, and the reported step names it. Stage C focus guard: recalcTarget takes a focused flag and returns null when the tab is focused, mirroring the #27 NewEntryPill policy — a focused/mid-edit view is never yanked (which would discard unsaved code); a backgrounded view still refreshes. seenRecalc seeds from the mount-time version so a pre-mount recalc isn't replayed. Plus: pin the companion auto path's by-design None checkpoint_step in test_companion_revise_publishes_recalc_event (assertion-tightening, green today). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5134347 commit 07d42b3

4 files changed

Lines changed: 32 additions & 11 deletions

File tree

packages/app/src/pages/CatalogPage.tsx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -346,15 +346,18 @@ export function CatalogPage() {
346346
}, [project, version]);
347347

348348
// A recalc re-points aliases to recomputed hashes; if the open entry view is
349-
// one of them, navigate to the new hash so the detail pane (keyed on the URL
350-
// hash) refetches the fresh result. Process each SSE version at most once
351-
// (StrictMode double-fires effects), mirroring NewEntryPill.
352-
const seenRecalc = useRef(0);
349+
// one of them (and this tab is backgrounded — see recalcTarget), navigate to
350+
// the new hash so the detail pane (keyed on the URL hash) refetches the fresh
351+
// result. Process each SSE version at most once (StrictMode double-fires
352+
// effects), mirroring NewEntryPill. Seed `seen` with the mount-time version so
353+
// a recalc that fired before this page mounted isn't replayed on first render.
354+
const seenRecalc = useRef(version);
353355
useEffect(() => {
354356
if (version === seenRecalc.current) return;
355357
seenRecalc.current = version;
356358
if (!project || !lastEvent || lastEvent.kind !== "recalc") return;
357-
const target = recalcTarget(hash, lastEvent.remap as Record<string, string> | undefined);
359+
const focused = typeof document !== "undefined" && document.hasFocus();
360+
const target = recalcTarget(hash, lastEvent.remap as Record<string, string> | undefined, focused);
358361
if (target && target !== hash) navigate(`/${project}/catalog/${target}`);
359362
// eslint-disable-next-line react-hooks/exhaustive-deps
360363
}, [version]);

packages/app/src/recalcRefresh.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,23 @@
11
// An auto-recalc (or explicit recalc) re-points an alias from an old content
22
// hash to a freshly recomputed one and publishes a `recalc` SSE event carrying
33
// the {oldHash: newHash} remap. The open entry view is addressed by content hash
4-
// in the URL, so a view parked on a head that was just remapped must navigate to
5-
// the recomputed hash to refresh. Only that view moves: an unrelated or older
6-
// hash (not a remap key) stays put. The navigation glue lives in CatalogPage.tsx.
4+
// in the URL, so a backgrounded view parked on a head that was just remapped
5+
// navigates to the recomputed hash to refresh. Only that view moves: an
6+
// unrelated or older hash (not a remap key) stays put. The navigation glue lives
7+
// in CatalogPage.tsx.
8+
//
9+
// A *focused* view stays put even when remapped — mirroring the NewEntryPill /
10+
// #27 policy of never yanking an active user. This also protects an in-progress
11+
// code edit: navigating remounts the hash-keyed detail pane and would discard
12+
// unsaved text. The common case (revise via Claude, the browser backgrounded)
13+
// still refreshes; only an actively-watched tab is left to refresh on its own.
714

815
// Returns the hash the open view should navigate to, or null when nothing moves.
916
export function recalcTarget(
1017
currentHash: string | undefined,
1118
remap: Record<string, string> | undefined,
19+
focused: boolean,
1220
): string | null {
13-
if (!currentHash || !remap) return null;
21+
if (focused || !currentHash || !remap) return null;
1422
return remap[currentHash] ?? null;
1523
}

src/tallyman_mcp/server.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ def wrapped(*args, **kwargs):
129129
"catalog_diff",
130130
"catalog_scan_staleness", # read-only staleness scan
131131
"catalog_recalc", # self-checkpoints (arg-aware: dry-run takes none, a real run one)
132+
"catalog_promote_diff", # self-checkpoints one tx for the promote + cascade (no double-commit)
132133
"catalog_list_summary_stats",
133134
"catalog_list_post_processings",
134135
"catalog_run_post_processing", # preview, persists nothing
@@ -909,7 +910,12 @@ def _resolve(v):
909910
# checkpoint so that one revision sweeps the promote + the cascade together.
910911
recalc_report = _auto_recalc_after_head_advance(project, target_alias) if repointed else None
911912

912-
checkpoint_catalog(project, f"tallyman: catalog_promote_diff {name} V{a_idx}→V{b_idx}")
913+
# promote_diff is checkpoint-exempt (self-checkpoints): THIS is the single
914+
# revision the promote + cascade land in. Stamp its step onto the recalc
915+
# sub-report — the checkpoint-free walk built it before this commit existed.
916+
step = checkpoint_catalog(project, f"tallyman: catalog_promote_diff {name} V{a_idx}→V{b_idx}")
917+
if recalc_report is not None:
918+
recalc_report["checkpoint_step"] = step
913919
_notify("entry_added", content_hash=result.content_hash)
914920
out = {
915921
"alias": target_alias,

tests/test_recalc_surfaces.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,5 +364,9 @@ def test_companion_revise_publishes_recalc_event(project, orders_parquet, monkey
364364
assert r.status_code == 200, r.text
365365
# The companion publishes the canonical recalc SSE event with the cascade's remap.
366366
assert captured, "no recalc event published"
367-
remap, _step = captured[0]
367+
remap, step = captured[0]
368368
assert remap == {b: get_alias(project, "b")}
369+
# checkpoint_step is None by design on the companion auto path: the middleware
370+
# commits the revision AFTER the handler returns, and (unlike the MCP decorator)
371+
# the response is already serialized, so there's nothing to backfill into.
372+
assert step is None

0 commit comments

Comments
 (0)