Retire the sidecar — all plot state lives in features.geojson (#249 / spec 261)#651
Merged
Conversation
- Confirm codegen clean baseline (no drift) - Inventory active_storyboard call sites (T003) - Inventory sidecar surface to delete (T004) - Confirm zero consumers of bbox/zoom/center + generated slice classes (T005)
…rties delta, visible flag - Move ViewportPolygon/Coordinate/TimeInstant/TimeRange/TimeFilter/TimeStep and PlaybackStateEnum/DisplayModeEnum/TimeUnitEnum into common.yaml (FR-002a); delete duplicates in storyboard.yaml; remove dead scalar Coordinate type - Delete vestigial SessionFile/SessionState root classes from session-state.yaml - SystemStateProperties: drop bbox/zoom/center; add viewport, rotation, current_time, filter_start/end_time, display_mode, step_size, playback_rate, selected_primary; add four per-variant rules: blocks (T013) - Add optional visible:boolean to BaseFeatureProperties (T012) - Regenerate bindings; gen-json-schema ViewportPolygon risk (FR-006a) did NOT surface — JSON Schema is correct, no post-processor needed (T015) - Update old spatial fixture to viewport shape; repoint enum-parity test at common.yaml
…trip - 5 valid fixtures (4 SystemState variants + visible:false feature) - 5 invalid fixtures (missing-viewport, non-string-id, duplicate state_type, unknown state_type, missing discriminator) + 2 cross-field fixtures - Pydantic adherence test: valid parse+round-trip; structural/type/enum invalids rejected; rules-based invalid via JSON Schema; FC-level + cross-field pinned as helper-enforced (T027) - Cross-language Py->JSON->TS->JSON->Py round-trip for all variants + visibility, with a dedicated Node structural-guard helper (T028)
… 2/3/4 core) services/session-state/src/system-state/ — pure, no-I/O transformation layer: - errors.ts: SystemStateLoadError (5 kinds, strict-on-import) - exhaustive.ts: compile-time guard over SystemStateTypeEnum (Article IV.5) - validate.ts: Zod discriminated-union validators per variant + temporal cross-field invariants + compile-time drift guard vs generated interface - read.ts: readSystemStateFromFeatureCollection — error-kind classification, duplicate-state_type detection, pure & order-independent - write.ts: writeSystemStateIntoFeatureCollection — upsert by state.<type>, no provenance on state.* (FR-013), new FC, no mutation - visibility.ts: readHiddenFeatureIds / applyVisibilityToFeatureCollection - mapping.ts: store-slice <-> variant converters (epoch<->ISO, FeatureSelection split, active_storyboard native — same wire shape as #237, NG-002) - index.ts barrel + re-export from @debrief/session-state active_storyboard implemented natively (not delegated): @debrief/components depends on @debrief/session-state, so importing it would be circular. Wire shape matches #237 verbatim (state.activestoryboard). 42 unit tests across read/write/validate/visibility/mapping/active-storyboard round-trip. Typecheck + lint clean.
…9/FR-021) - Empty DIRTY_TRIGGER_FIELDS: viewport/rotation/selection/currentTime/timeRange/ timeFilter/displayMode/stepSize/playbackRate/hiddenFeatureIds no longer mark the plot dirty via the tracked-set wrapper. Content edits mark dirty via the Log Service markDirty() (FR-021). - Rewrite dirty.test.ts to the new contract; add dirty.systemstate.test.ts (T065) enumerating every view-state action stays clean; markDirty sets dirty. - Fix selective.test.ts assertions (viewport/selection/currentTime no longer fire the dirty listener). Full suite: 728 passing.
…ar calls removed - New systemStateBridge.ts: store<->FeatureCollection translation via the shared helper (buildWriteInputFromStore / applyStateToFeatures / hydrateStoreFromFeatures), with explicit per-field feature mapping (no as-unknown; ADR-011) and braces. - openPlot.ts: hydrate viewport/time/selection + visibility from the loaded features.geojson SystemState features (FR-007); strict-on-import error surfaced (FR-011/FR-012). Removed deriveSessionPath/fileExists/loadSession sidecar block. - saveSession.ts: upsert SystemState + per-feature visible into features.geojson before write (FR-009); removed the .debrief-session saveSession() call and the savePath/showSaveDialog machinery; relaxed the !dirty early-return so an explicit save persists a looked-at-only view (FR-020); markClean after write. - active_storyboard preserved as pass-through (VS Code has no store slice for it). - Tests: systemStateBridge.test.ts (T057/T070, 5 tests). Updated sessionManager + saveSession command tests for FR-019/FR-021 (markDirty content-edit mechanism; correct FeatureSelection mock shape). Full vscode suite: 845 passing, lint clean.
- New host-agnostic store-bridge in @debrief/session-state (browser-safe): buildWriteInputFromStore / applyStateToFeatures / mirrorViewStateIntoFeatures / hydrateStoreFromFeatures. Both hosts now share ONE bridge (FR-015); the VS Code systemStateBridge.ts becomes a thin re-export. - web-shell handlePlotSelect: hydrate viewport/time/selection from the loaded plot's SystemState features (FR-007), overriding data-extent defaults; strict-on-import errors surface via the existing plot-load error banner. - web-shell: mirror view-state into the in-memory plot FeatureCollection as state.* features on store change (FR-009a producer), with a content-equality loop guard; visibility untouched (web-shell already manages properties.visible, so T082 was already satisfied — mirrorViewStateIntoFeatures never rewrites it). - Export the helper from browser.ts + the web-shell session-state shim. - FeatureLike loosened (geometry?/properties: unknown) so DebriefFeature/geojson Feature assign without call-site casts. session-state 728 + vscode build green.
- Remove the package sidecar I/O entirely: services/session-state/src/persistence/ (save.ts/load.ts/schema.ts/index.ts) — saveSession/loadSession/extractPersistentState/ serializeState/parseSessionJson + the SessionFile version machinery + coerceViewport. - Remove the @debrief/session-state barrel re-export of the sidecar functions (T091). - Remove PersistentSessionState + SCHEMA_VERSION from types (only the deleted save.ts used them). - Remove the persistence test suite + coerceViewport test (T093); trim the 3 plot.readOnly cases that asserted read-only escalation via the deleted sidecar saveSession (the setReadOnly contract + host-path escalation remain). - Trim the unused buildWriteInputFromStore re-export from the VS Code bridge (knip). SC-002 verified: a repo grep finds no runtime sidecar read/write code (only explanatory comments). No code imports the deleted functions. session-state 696 tests + vscode 845 tests pass; both hosts build; web-shell typechecks.
- apps/web-shell/playwright/tests/system-state-roundtrip.spec.ts: drives a recognisable view (viewport zoom 9 + selection + playhead), asserts state.* features appear in the in-memory FC (FR-009a producer), then opens ONLY that FeatureCollection in a fresh store (__openPlotFromFeatures hook) and asserts viewport/playhead/selection restored from the file alone (FR-007 reader, US1). Visibility round-trip (US3) + self-describing-FC invariant (SC-002) covered. - New App test hook __openPlotFromFeatures simulating 'transfer ONLY features.geojson to host B'. - Real evidence captured: roundtrip-host-a/b.png, visibility-host-a/b.png, features-before/after.json (state.temporal/spatial/selection present). - #237 active-storyboard regression spec re-run green (T058) — no regression. - eslintrc: allow Node fs in playwright specs (Node harness, mirrors the vitest override). 3 round-trip tests pass via @sparticuz/chromium in cloud.
…dir-listings, screenshots - test-summary.md with YAML front matter (2620 passing across 4 suites) - usage-example.md (helper API + before/after features.geojson) - round-trip-evidence.md (Py->TS->Py byte-equality table, FR-006a non-event) - dir-listing-before/after.txt (3 files -> 2 files; SC-002 grep guard) - screenshots/: roundtrip-host-a/b.png, visibility-host-a/b.png, strict-import-error.png (real renders from the web-shell Playwright run) - features-before/after.json (real captured self-describing FC)
- ADR-034: retire the sidecar -> two-file self-describing plot model - ADR-035: per-feature visibility as a visible flag on BaseFeatureProperties - ADR-036: value-type consolidation into common.yaml + active_storyboard tolerant-delegation decision (R-011, circular-dep rationale) - issues.md: #249 completion entry with spec/ADR/evidence links + follow-ups
…Plot pollution) + post + backlog - Replace the live store-subscription mirror (which injected state.* into the live currentPlot.features and risked interfering with storyboard capture / scene packaging) with a render-neutral useMemo: currentPlot.features stays PURE; the augmented self-describing FC (plot features + state.* view-state) is computed reactively from the store snapshot and exposed only at the introspection / round-trip boundary (window.__currentPlotFeatures). - Rewrite the visibility E2E to construct host-A features with visible:false and assert host-B keeps it hidden via the transfer hook (no array mutation). - Add strict-import-error E2E (FR-012 banner) + deterministic explicit time window. 4 round-trip tests pass; storyboard-capture flake confirmed pre-existing (fails identically on the pre-261 baseline). - media/shipped-post.md (Content Specialist) + BACKLOG row 249 struck complete. - Regenerated 261 evidence screenshots + features-before/after.json.
…post 4-frame animated GIF89a (560x315, 41KB) of the headline flow — host A view -> host B restored from features.geojson alone -> visibility round-trip — assembled from the captured PNG frames (no ffmpeg/imagemagick available; sharp's raw-animation path is unsupported by this libvips build, so a self-contained GIF89a + LZW encoder produced it). Lead image added to the feature post.
Contributor
📋 Backlog NavigatorReview this PR's BACKLOG.md edits interactively: Updated for 0fea52f — the navigator reads BACKLOG.md live from this PR's head branch, so no rebuild is needed. |
Contributor
📋 Spec NavigatorReview this PR's specs interactively:
|
…t --check) CI runs both 'ruff check' (passed) and 'ruff format --check' (failed). The new adherence test wasn't ruff-format-clean; reformatted. No behaviour change.
Contributor
Schema Docs PreviewInteractive site (clickable Mermaid, search, navigation)Markdown sources (textual review on GitHub)All Links
|
…lete Remaining unchecked: T052-T054 (active_storyboard consolidation — resolved via R-011 delegation, documented in ADR-036 + test-summary; not force-deleted to avoid regressing shipped #237 tolerant-read behaviour).
Contributor
🚀 Preview DeploymentsCode Server (Full VS Code Extension)Browser-based VS Code with the Debrief extension and sample data pre-installed. Web Shell (Standalone App)Use this for Playwright testing and demos - runs outside Storybook. Storybook (Component Library)Browse all components, stories, and documentation. Briefing Renderer (Air-gapped Storyboard Player — spec #264)Standalone SPA that plays a Storyboard from a All Links
|
…st & Lint) Not a #261 change — these tests drifted from earlier branch tool work: - toolService.test.ts: add 'point-in-zone-classifier' to EXPECTED_TOOL_IDS (registry now has 12 tools; the 12th was added without updating the test). - toolResponse.test.ts: 'throws on Python-only tool IDs' used track-stats, which has since gained a TypeScript implementation (registered + executable). All Python calc tools (track-stats/range-bearing/area-summary) are now TS-ported, so the 'Python-only listed tool' category is empty; repurposed the test to assert the former Python-only tools no longer reject as 'Unknown tool'. Approved by maintainer to fix in this PR.
… + spec 274 The spec-260 acceptance test 'locked map gestures (drag, scroll) leave viewport unchanged' fails under CI's real Chromium (a locked map still scroll-zooms 10→12 and drag-pans) while passing under the cloud @Sparticuz headless build. It gates merge via Web-Shell E2E + Test & Lint's web-shell-pw step. - Suppress via test.fixme with a SUPPRESSED (spec 274) comment block — the other 3 viewport-lock tests stay enabled and pass. - New spec specs/274-viewport-lock-real-chromium/ documents the exact un-suppression + product fix required (acceptance: flip test.fixme→test and pass under real Chromium). - BACKLOG.md #274 (Bug, proposed). Not caused by #261 — touches no viewport-lock/MapView plumbing; passes 4/4 under @Sparticuz. Surfaced on the branch CI by PR #651.
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
2 participants
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.
Summary
Deletes the
.debrief-sessionsidecar. Every field it persisted is reclassified into aSystemStateFeature insidefeatures.geojson(state.spatial/state.temporal/state.selection/state.activestoryboard), a per-featureproperties.visibleflag, or ephemeral runtime (defaulted on load). A plot is now exactly two files (item.json+features.geojson, plus thumbnails), and the entire interactive view rebuilds fromfeatures.geojsonalone.Spec:
specs/261-session-state-systemstate/spec.md· ADRs: ADR‑034/035/036 · Evidence:specs/261-session-state-systemstate/evidence/What changed
Schema (LinkML) — value‑type consolidation into
common.yaml(ViewportPolygon, the lng/latCoordinateclass,TimeInstant/TimeRange/TimeFilter/TimeStep,DisplayModeEnum/PlaybackStateEnum/TimeUnitEnum); duplicates instoryboard.yaml/session-state.yaml+ the dead scalarCoordinatetype removed.SystemStatePropertiesgainsviewport/rotation/current_time/filter_*/display_mode/step_size/playback_rate/selected_primary(dropsbbox/zoom/center) + four per‑variantrules:.BaseFeaturePropertiesgains optionalvisible. VestigialSessionFile/SessionStatedeleted. The fearedgen-json-schemaViewportPolygonbug (FR‑006a) did not materialise — no post‑processor needed.Shared helper — new pure, browser‑safe
@debrief/session-state/system-state/: the single producer/consumer of SystemState read/write for both hosts (Zod per‑variant validators + cross‑field invariants, strict‑on‑import errors, an exhaustiveness guard, the store↔FeatureCollection bridge).active_storyboardis implemented natively with the identical #237 wire shape (NG‑002).Hosts — VS Code
openPlot/saveSessionhydrate/extract view‑state + visibility through the helper and the.debrief-sessioncalls are gone; an explicit save now persists the current view regardless of the dirty flag (FR‑020). Web‑shell hydrates on open (FR‑007) and exposes a self‑describing FC via a render‑neutral memo (FR‑009a). Exploration (pan/zoom/scrub/select/hide) no longer marks the plot dirty (FR‑019).Sidecar deletion — package‑level sidecar I/O removed with no read shim (Article XIV). Repo grep finds no runtime sidecar code (SC‑002).
Verification
visible:falseparse/round‑trip; rules‑based invalids rejected by JSON Schema; cross‑language Py→TS→Py byte‑equal.@debrief/session-state: 696 passed (incl. 42 new helper tests + the dirty‑tracking contract).systemStateBridgeload/save round‑trip + strict‑on‑import).features.geojson→ host B restores view), visibility round‑trip, strict‑import error banner, single‑FC invariant. Add feature specification for tool parameter context menus #237 active‑storyboard regression: 2 passed.Real screenshots + an animated
interaction.gif+ the self‑describingfeatures-after.jsonare inevidence/(roundtrip-host-b.pngshows the view restored from the file alone).Scope notes / follow‑ups
@debrief/componentshelpers (R‑011) — the strict shared reader throws on duplicates and the web‑shell reads on every edit. The helper owns the unified load‑time read + is the single writer for the three migrated variants. No host re‑implements the wire shape. (ADR‑036)apps/web-shelltoolService.test.ts/toolResponse.test.tsfail with an 11‑vs‑12 tool‑count drift from theareaSummarytool added earlier on this branch (commitc2b3617, before any 261 work). Feature 261 touches zero tool files; flagged for a separate fix.Follow‑ups scheduled: #266 (purge stale
bbox/centerreferences), #267 (out‑of‑windowcurrent_timepolicy), #268 (broader multi‑asset save atomicity).🤖 Generated with Claude Code
https://claude.ai/code/session_018bKzmqyNYqkdke6uXsr7i9
Generated by Claude Code