docs(skills): ui-capabilities — view state also dies to a re-fire, with nothing unmounting - #489
Merged
Merged
Conversation
…th nothing unmounting (cf. #482) The skill records that a query key naming a mutable value is an unmount trigger. #482 was dispatched against that rule and the query key turned out to be innocent: `AnnotatorCanvas`'s initial-fit layout effect re-fired because `fit` depended on the `AssetDescriptor` object, which `documentFromWire` mints afresh on every rebuild — so a save's refetch reset zoom and pan in a component that never remounted. The hunt for an unmount that never happened is the cost of the two mechanisms not being written down together. They are adjacent now, with the tell that separates them: sibling state in the same component. An unmount takes all of it and flashes a loading state on the way; a re-fire disturbs only what that one hook writes and leaves everything beside it untouched. Also records the two habits that follow — depend on the values a hook is really a function of rather than the object carrying them, and put the primitives in the callback's dependency list rather than the effect's, since `react-hooks/exhaustive-deps` is an error in `frontend/annotator` and refuses a widened list. The skill's `description` gains the state-lifetime clause, so the next agent debugging a silently reset piece of view state finds this file by searching for what they are actually looking at. cf. #485.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documentation only — one skill file, +15 −1.
What this records
The skill already carries "a query key that names a value the page itself can change is an
unmount trigger" (#379, cf. #368). #482 was dispatched against that rule and the query key
turned out to be innocent: the viewport reset had nothing unmounting at all. The two
mechanisms are the confusable pair, so they now sit next to each other.
Written from the fix rather than from a summary of it —
git show 9135411 -- frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx:assetissnapshot.document.asset;documentFromWiremints a freshAssetDescriptoronevery rebuild; a save's refetch rebuilds. So
fit's identity moved, the layout effect thatseeds zoom and pan ran again, and the camera went back to the fitted view — in a component
that never remounted and with no key having changed. The repair anchors the identity to the
three numbers a fit is actually a function of.
The three things the new section adds
re-fires its consumer, and a re-fire resets state exactly as an unmount does — with no
unmount to find. Nothing remounts, no key moves, no loading state flashes.
saved Saving resets the editor viewport (zoom/pan) #482 a wrong turn: sibling state in the same component. An unmount takes all the
local state in that subtree and flashes a loading state on the way; a re-fire disturbs
only what that one hook writes. In Saving resets the editor viewport (zoom/pan) #482 the hidden-annotation set, the interaction state
and the hover point all survived the reset that took the viewport.
refetch returning identical JSON returns the same array and nothing re-fires — only a
write ever trips it. The reset therefore looks like a consequence of saving, and the
search goes to the mutation, which is innocent.
Plus the two habits that follow: depend on the values a hook is really a function of rather
than the object carrying them, and put the primitives in the callback's dependency list
rather than the effect's —
react-hooks/exhaustive-depsis anerrorinfrontend/annotatorand refuses a widened list, which is the correct refusal.Deduplication
The rule was briefly written to this agent's private memory while #485 was in flight. That
file has been deleted and the shipped-work record it belonged to now points at this section
instead: a rule kept outside its home rots into misinformation, and the next agent hunting a
silently reset piece of view state will search the skill, not another tool's storage. One
detail that existed only in that note — the
exhaustive-depsconstraint above — was carriedinto the skill before the note was removed, so nothing was lost.
A repo-wide sweep found no other copy:
docs/,CLAUDE.md,AGENTS.md, the other skillsand the
tests/tree contain nothing about hook dependency identity. PR #485's body is thehistorical record of the fix and is untouched.
The skill's
descriptiongains a state-lifetime clause. It described only capabilities andrefusals, so neither this section nor the query-key one above it was discoverable by
description — an agent debugging state a refetch reset had no reason to open the file.
Scope
No code, no tests, no wire, no generated artifacts.
.claude/skills/and.cursor/skills/are git-ignored symlinks to this path, so both tools pick the change up with no setup re-run.
Local gate
Full run, staged against this box's ~10-minute command ceiling, pytest split by directory
derived from
ls tests/at run time. Run in full despite the markdown-only diff, per thedispatch. Every stage's exit code:
pytest tests/architecture0pytest tests/cli0pytest tests/examples0pytest tests/fixtures0pytest tests/formats0pytest tests/inference0pytest tests/jobs0pytest tests/kernel0pytest tests/mcp0pytest tests/packaging0pytest tests/scripts5pytest tests/server0pytest tests/test_versioning.py0ruff check .0ruff format --check .0mypy src/visionset/kernel0lint-imports0check.sh frontend generated0check.sh browser0— 238 e2e passed, 1 cycle passedtests/scriptsexits 5 by design — nothing pytest-shaped lives there; it isnode --testand runs under
check.sh generated. That group is the one with a genuine stake in thisdiff:
docs_links.test.mjsscans every tracked*.md, skills included.cf. #482, cf. #485.