feat: refresh the viewport by itself when the store changes - #401

PR #401 - viewport self-refresh on store change - CHANGES REQUESTED

One required change, inline: the commit carries two files from the still-unmerged #242. On the base master there is no skills/ tree at all - skills/beamcraft-model/reference/view.md and skills/open/SKILL.md exist only on the #242 branch, and here they arrive as a partial copy (the reference file without its SKILL.md, the open skill without the plugin manifests). If this merges first, master gains orphan docs for a plugin it does not have; if #242 merges first, the same lines conflict across the two branches. Drop both files from this PR: the auto-refresh sentence belongs in #242 (or a tiny docs PR after both merge), and the apps/beamcraft-ui/README.md note - the only doc that exists on master - already covers the feature here.

The watcher itself reviewed clean:

• revisions_stamp is a directory mtime read; a missing revisions directory is None (no signal, no error), and the first commit into an empty store moves None to Some - both pinned by the new test.
• spawn_watch polls a stat every 500 ms on one unnamed-workspace thread, exits when the receiver drops, and a failed start only costs the automatic reload - R keeps working; the right ladder rung versus a notify dependency.
• The signal feeds the same reload path as the R key, so the existing debounce (reload_queued) and the framing-driven refit apply unchanged; extra queued messages collapsing into one refresh is the wanted behaviour.
• Stamp granularity and GC: a history compact that prunes old revision files also moves the stamp and fires one harmless reload of an unchanged head.
• README note matches the behaviour.

With the two files dropped, this is an approve.

PR #401 - viewport self-refresh on store change (round 2) - APPROVED

The round-1 finding is resolved: the branch merged upstream master, so skills/beamcraft-model/reference/view.md and skills/open/SKILL.md now come from the base (#242 is merged there) and the PR delta on them is only the two auto-refresh sentences. No orphan files, nothing duplicated, and the merge dropped nothing from the feature (the watcher files are byte-identical to the round-1 head, which reviewed clean: stamp semantics, polling thread lifecycle, shared reload path with its debounce and framing-driven refit, GC-triggered stamp moves firing one harmless refresh).

No change requested. Process note, body only: the branch still carries the unmerged #243 as its first commits, as the description discloses, and upstream master has since moved (#241); after #243 merges, the promised rebase collapses this diff to the feature plus the two doc sentences.

PR #401 - viewport self-refresh on store change - APPROVED (re-request, unchanged head)

The head is the same commit (5d6eafd) this reviewer approved minutes ago (GitHub review 5643274150): the watcher feature, its test, the README note and the two skill-doc sentences, with the upstream merge bringing #242 into the base. No new commits, no new evidence, verdict unchanged: approved, no change requested.

Reviewed d959e9d against a17e694 (4 files: apps/beamcraft-ui/src/app.rs plus the three doc files). Approving — no changes requested.

The design is right for the job: the watcher polls the revisions/ directory stamp every 500 ms instead of pulling in a platform FS-event dependency, and a commit is exactly one new hard-linked record file appearing in that directory (crates/host/src/revision.rs), so a moved directory mtime is a faithful "the head may have moved" signal. A missing directory resolving to None is treated as no signal, so an empty store stays calm until the first commit creates it.

Verified:

• store_changed routes through the same self.reload() as the R key, so an in-flight reload queues via reload_queued (app.rs:707) and the framing-change refit comes from the existing needs_refit path in poll_reload.
• The thread unwraps nothing and exits on its own when the window closes (tx.send errors once the receiver drops); a watcher that cannot start degrades to the manual R only, as documented.
• A stamp move from .tmp-* crash leftovers or from the UI committing to its own store costs at most one extra reload that source::reload resolves to Refreshed::Unchanged (snapshot-token dedupe), so the camera is kept.
• The new test matches the repo fixture convention (pid + nanos in the temp name) and pins both documented behaviors: no directory = no signal, a new record file moves the stamp.

Two non-blocking observations, both fine as is:

• A commit landing in the tiny window between the initial load and the watcher first stamp read is missed until the next commit (or R); closing it would mean capturing seen on the main thread before spawning, not worth the coupling.
• Only one queued message is consumed per frame, so a burst of commits costs one extra reload on the next frame — converges, harmless.

The doc updates (ui README, skills view/open) match the implemented behavior, including the honest "accepted by the person looking at it" caveat.

Please address these two issues before merging:

1. [P2] Detect the committed head rather than relying solely on directory mtime. revisions_stamp() compares only metadata.modified(). Filesystems with coarse timestamp resolution can give successive directory changes the same timestamp, so a completed commit can remain invisible to the viewport until another change or a manual reload. Please compare the committed head identity/sequence (or another reliable revision token). The new test also assumes that a 10 ms sleep changes the timestamp; please add coverage that does not depend on timestamp precision and verifies that a changed head triggers a reload.

2. [P2] Stop the watcher when its owning model/window is dropped. The worker discovers that its receiver has been dropped only when tx.send() fails, and it sends only after the stamp changes. If the old store remains unchanged, the detached thread keeps waking and checking the filesystem indefinitely. Repeated model opens can accumulate these workers. Please add an explicit shutdown mechanism tied to the owner's lifetime, and test that the watcher exits even when no filesystem change occurs.

The existing asynchronous reload path is reused appropriately, and CI is green at d959e9d21b06d77894b1a1b7c9bc1427060090ad. These requests concern reliable change detection and watcher lifetime; I have not run the native UI locally.

Great work, Thank you