skills(sightmap): collect view/request memory per spec §Memory#62
skills(sightmap): collect view/request memory per spec §Memory#62chiplay wants to merge 1 commit into
Conversation
collect_memory only walked the top-level `memory:` field, so memory written under a view or under a request was silently dropped on upload. Since sightmap_update_view writes memory under each view (the natural scope when the corpus is keyed by view), every agent-curated entry was invisible to the live snapshot pipeline. Sightmap v1 spec §Memory explicitly allows memory at four scopes — file, view, component, and request — and the canonical example (flights.yaml) exercises all of them. parse_file already iterates view-scoped components; this mirrors the same pattern for memory at file/view/request scope. Component memory continues to be carried on each component entry by flatten_components. Adds a testdata fixture (memory.yaml) covering all four scopes plus eight new tests under TestCollectMemory, including a negative assertion that component memory does NOT flatten into the top-level list. Note: two pre-existing unrelated test failures in TestFindSightmapFiles / TestCollect (nested-.sightmap/ discovery) are present on origin/main and not addressed here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #62 —
|
| Scope | Spec says flatten? | main |
fix-collect-memory-spec |
|---|---|---|---|
file (top-level memory:) |
yes | ✓ | ✓ |
view (views[*].memory:) |
yes | ✗ dropped | ✓ |
request (requests[*].memory: and views[*].requests[*].memory:) |
yes | ✗ dropped | ✓ |
component (views[*].components[*].memory:) |
no — stays on the component | ✓ (not flattened) | ✓ (not flattened) |
Function under test
collect_memory(root: str) -> list[str] walks <root>/.sightmap/*.yaml and returns the memory entries that should be flattened into the upload payload's top-level memory[] field. Component memory rides along on the component, not in this list.
Per-scope evidence
Each section feeds collect_memory a single yaml file isolating one spec scope, then prints the return value on each branch. Same INPUT → diverging OUTPUT is the whole story.
1. File scope — baseline, should work on both
# .sightmap/fixture.yaml
version: 1
memory:
- Dates throughout the app are ISO-8601 (YYYY-MM-DD)>>> collect_memory(root)// MAIN
["Dates throughout the app are ISO-8601 (YYYY-MM-DD)"]
// BRANCH
["Dates throughout the app are ISO-8601 (YYYY-MM-DD)"]✓ Backwards-compat: pre-existing file-level convention is unchanged.
2. View scope — the bug
# .sightmap/fixture.yaml
version: 1
views:
- name: search
route: /search
memory:
- Hitting Enter inside the date input submits without clicking Search>>> collect_memory(root)// MAIN
[]
// BRANCH
["Hitting Enter inside the date input submits without clicking Search"]✗→✓ This is what every entry written by sightmap_update_view looks like. main silently drops it.
3. Request scope — same bug, second axis
# .sightmap/fixture.yaml
version: 1
requests:
- name: Health
method: GET
path: /healthz
memory:
- Returns 503 during deploys; clients should treat that as not-yet-ready>>> collect_memory(root)// MAIN
[]
// BRANCH
["Returns 503 during deploys; clients should treat that as not-yet-ready"]✗→✓ Top-level requests and view-scoped requests both flow through the new code path.
4. Component scope — the negative invariant
# .sightmap/fixture.yaml
version: 1
views:
- name: search
route: /search
components:
- name: SearchForm
selector: form.search
source: src/Search.tsx
memory:
- Accepts typed YYYY-MM-DD — skips the calendar>>> collect_memory(root)// MAIN
[]
// BRANCH
[]✓ Component memory must not flatten — it ships on the component via flatten_components. Both branches agree; this PR doesn't regress the invariant.
End-to-end on the canonical fixture
skills/shared/testdata/.sightmap/memory.yaml exercises all four scopes simultaneously (file + view + request + view-scoped-request + component). Same fixture, both branches:
>>> collect_memory("testdata-root-containing-memory.yaml")// MAIN — 2 of the 6 flatten-eligible entries; 4 silently dropped
[
"Dates throughout the app are ISO-8601 (YYYY-MM-DD)",
"All currency values are USD minor units (cents)"
]
// BRANCH — all 6 flatten-eligible entries present; component entry correctly absent
[
"Dates throughout the app are ISO-8601 (YYYY-MM-DD)",
"All currency values are USD minor units (cents)",
"Returns 503 during deploys; clients should treat that as not-yet-ready",
"The search form lives inside a modal on mobile; selectors differ",
"Hitting Enter inside the date input submits without clicking Search",
"Rate-limited to 10 requests/min per user; returns 429 beyond that"
]Note what's not in the BRANCH output: "Accepts typed YYYY-MM-DD — skips the calendar". That's the component-scoped entry; it stays attached to SearchForm in the components stream.
Reproduce
The proof doc above was generated by this script. Copy-paste to re-verify on your machine.
import subprocess, importlib.util, json, os, tempfile, shutil
REPO = "/path/to/subtext" # adjust
WT = f"{REPO}/.worktrees/fix-collect-memory-spec"
def load(path, name):
spec = importlib.util.spec_from_file_location(name, path)
mod = importlib.util.module_from_spec(spec); spec.loader.exec_module(mod)
return mod
# main version of the file via git show, into a temp .py
main_src = subprocess.check_output(
["git", "-C", REPO, "show",
"main:skills/shared/collect_and_upload_sightmap.py"], text=True)
main_path = tempfile.NamedTemporaryFile("w", suffix=".py", delete=False).name
open(main_path, "w").write(main_src)
m = load(main_path, "m")
b = load(f"{WT}/skills/shared/collect_and_upload_sightmap.py", "b")
def run(yaml_src):
d = tempfile.mkdtemp(); os.makedirs(f"{d}/.sightmap")
open(f"{d}/.sightmap/fixture.yaml", "w").write(yaml_src)
return m.collect_memory(d), b.collect_memory(d)
# point at the real fixture for the end-to-end run:
fixture = open(f"{WT}/skills/shared/testdata/.sightmap/memory.yaml").read()
before, after = run(fixture)
print("MAIN :", json.dumps(before, indent=2))
print("BRANCH:", json.dumps(after, indent=2))What a reviewer should check
- The
MAIN []lines in §2 and §3 — that's the bug, in one byte of evidence. - The
BRANCH []line in §4 — that's the invariant the PR preserves: component memory does not leak into the flat list. fixtureparity in §End-to-end — the same yaml file goes in; only the OUTPUT differs. No fixture-shopping.
Summary
collect_and_upload_sightmap.collect_memoryonly walked the top-levelmemory:field, so memory written under a view or under a request was silently dropped on upload. Becausesightmap_update_viewwrites memory under each view (the natural scope when the corpus is keyed by view), every agent-curated entry was invisible to the live snapshot pipeline.The fix mirrors the pattern
parse_filealready uses for view-scoped components: iterateviews[*]and pick up memory at each scope.Why this is the right shape
Sightmap v1 spec §Memory is explicit:
The canonical
flights.yamlexercises all four scopes. This PR makescollect_memorymatch that contract:memory[]payloadflatten_components(unchanged)Repro before fix
On a
.sightmap/corpus where every entry was written viasightmap_update_view(so memory lives at view scope):After fix, on the same corpus: the 14 view-scoped memory entries upload as expected.
What's in this PR
skills/shared/collect_and_upload_sightmap.py—collect_memoryrewrite +_normalize_memoryhelperskills/shared/testdata/.sightmap/memory.yaml— new fixture exercising all four spec scopesskills/shared/test_collect_sightmap.py—TestCollectMemoryclass, 9 new tests including a negative assertion that component memory does not flatten into the top-level listTest plan
python3 -m pytest skills/shared/test_collect_sightmap.py— 33 passed, 2 pre-existing unrelated failures (see note below)Note on pre-existing failures
TestFindSightmapFiles.test_finds_nested_sightmapandTestCollect.test_collects_all_componentsfail onorigin/mainbefore this PR. They expectfind_sightmap_filesto discovertestdata/packages/dashboard/.sightmap/dashboard.yaml, but the function only walksroot/.sightmap/by design (intentional, per its docstring: "to avoid walking potentially massive directory trees"). These are orthogonal to this change and left untouched.🤖 Generated with Claude Code