From d313b97ee013250ee1d4757ec4a5dc677341c553 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Jun 2026 13:03:18 +0000 Subject: [PATCH] docs(schema): purge stale bbox/zoom/center SystemState-spatial refs (#266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tech-debt follow-up to #249/spec-261, which unified SystemStateProperties.spatial onto `viewport: ViewportPolygon` (+ optional `rotation`), removing the legacy `bbox: float[4]` / `zoom: float` / `center: float[2]` fields. Updates every remaining doc that still described the old shape: - shared/schemas/README.md — state.spatial variant line - specs/237-active-storyboard-persistence/data-model.md — SystemStateTypeEnum.spatial desc - specs/022-system-kind-discriminator/ (the spec that originally defined the shape): spec.md (acceptance criterion + Key Entities + a "superseded by #261" note), research.md (rationale), data-model.md (field table + validation rule), evidence/usage-example.md (Python snippet), evidence/sample-spatial.json (rewritten to the 4-corner polygon and re-validated against the live SystemState Pydantic model) Retained intentionally (different, current concepts): storyboard Viewport (center/zoom/bearing), STAC spatial.bbox, GeoJSON bbox members, ViewportPolygon.zoom. Marks BACKLOG #266 complete and logs the work in issues.md. https://claude.ai/code/session_019XNKPXw6XgxjrPx1nsGrNj --- BACKLOG.md | 2 +- docs/project_notes/issues.md | 5 +++++ shared/schemas/README.md | 2 +- specs/022-system-kind-discriminator/data-model.md | 7 +++---- .../evidence/sample-spatial.json | 13 ++++++++++--- .../evidence/usage-example.md | 15 +++++++++++---- specs/022-system-kind-discriminator/research.md | 2 +- specs/022-system-kind-discriminator/spec.md | 6 ++++-- .../data-model.md | 2 +- 9 files changed, 37 insertions(+), 17 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index d308dc746..9b338dd5b 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -178,7 +178,7 @@ Description formats: | ~~264~~ | ~~Feature~~ | ~~[Air-gapped briefing zip — self-contained distraction-free Storyboard renderer (SPA)](specs/264-briefing-zip-renderer/spec.md) — ship the briefing renderer as a self-contained zip that opens in any modern browser by double-clicking `index.html`: no server, no extension host, no network calls. Contents = bundled SPA + `features.geojson` (which already contains the `StoryboardFeature` / `SceneFeature` entries — Storyboards live inside the plot's GeoJSON per `storyboard.yaml`, not as a separate document) + `item.json` + Scene-thumbnail assets + pre-fetched basemap tiles. Build command takes a Storyboard ID (one plot may contain multiple Storyboards). Single Storyboard per zip, read-only, with Present (chrome-hidden) / Minimal (transport + scrub) toggle. Reuses the shared playback engine from #217 + #258 displayMode capture. New VS Code command "Export Storyboard as briefing zip…" on the Storyboard overflow menu. New SPA at `apps/briefing-renderer/`. **Blocked on #263** (time-range Scenes) per interview sequencing — briefing renderer should ship knowing time-range is in play to avoid rework on the shared engine. (split from #229 via `/interview 229`; requires #215–#218 MVP, #174 thumbnails, #258 displayMode capture; [issue #631](https://github.com/debrief/debrief-future/issues/631))~~ | ~~4~~ | ~~5~~ | ~~3~~ | ~~12~~ | ~~Medium~~ | ~~complete~~ | ~~E13~~ | ~~2026-05-19~~ | ~~2026-05-20~~ | | 268 | Tech Debt | VS Code save atomicity (broader) — `apps/vscode/src/commands/saveSession.ts:163–208` currently performs three separate write operations (sidecar at 163, FeatureCollection at 178, thumbnails at 184–203) with non-blocking error handling at lines 180 and 192–202. After #249/spec-261 lands FR-019, the FC↔sidecar pair on the *migrated-state* path is sequenced FC-first/sidecar-second to close the silent-failure gap for that specific path — but the broader flow (thumbnails, STAC assets, sidecar, FC) is still independently committable. A real production failure mid-sequence could leave any subset of files updated. This ticket scopes a broader transactional-save model: either (a) a save-orchestrator that stages all writes to a tmp directory and atomically moves them into place, or (b) an explicit "last good known state" recovery file so a half-completed save can be rolled back on the next open. Estimate 5–8 dev-days; medium-to-high complexity (touches every save consumer). Trigger: a user reports a partial-save corruption, OR a security/audit review flags the non-atomic save flow. (follow-up to #249; cross-references docs/project_notes/decisions.md re: writer abstraction; potentially extends Constitution Article IV.4) | - | - | - | - | High | approved | | 2026-05-19 | 2026-05-31 | | ~~267~~ | ~~Tech Debt~~ | ~~[Out-of-window `current_time` policy — #249/spec-261 introduces an optional `current_time` field on the LinkML `SystemStateProperties` `temporal` variant (the saved playhead position) and ships strict-on-import validation: if a plot's saved `current_time` falls outside its `[start_time, end_time]` window, the load fails with `SystemStateLoadError(kind='cross-field-invariant')`. This is the right Article XIV.4 default for v4.x pre-release. But if analyst feedback later shows that malformed plots arrive often enough to be a friction (e.g. a colleague trims the analytical window after scrubbing, leaving the playhead orphaned), the team may want a tolerant alternative: clamp to the nearest window edge with a non-modal toast, OR offer "open ignoring saved playhead" as a fallback action. This ticket revisits the policy if/when the strict-on-import behaviour proves user-hostile. Likely depends on real-world usage data from #249 ship. Estimate 1–2 dev-days. Trigger: analyst feedback that plots are failing to load due to out-of-window playheads, OR a deliberate UX decision post-v4.0.0 to add tolerant import paths (Article XIV trigger point). (follow-up to #249; depends on real-world data)](specs/267-tolerant-playhead-import/spec.md)~~ | ~~2~~ | ~~1~~ | ~~4~~ | ~~7~~ | ~~Low~~ | ~~complete~~ | | ~~2026-05-19~~ | ~~2026-05-29~~ | -| 266 | Tech Debt | Spatial-shape cleanup follow-up to #249 — #249/spec-261 unifies the LinkML schema's spatial-viewport representation onto `ViewportPolygon`, removing the parallel `bbox: float[4]` + `zoom: float` + `center: float[2]` fields from `SystemStateProperties` (per Article XIV.1 — verified zero runtime blast radius today). If any docs, ADRs, ARCHITECTURE.md sections, code comments, or external references still mention the `bbox`/`center` shape after #249 ships, they need updating. Likely affected: `docs/project_notes/decisions.md` (any ADR referencing the spatial SystemState shape), `ARCHITECTURE.md` schema-extension passages, MCP envelope docs if any, and any spec for #215 / #237 that referenced the bbox shape. Estimate 0.5–1 dev-day. Trigger: #249 has shipped and a grep for `SystemStateProperties.*bbox` / `state_type.*spatial.*bbox` over the repo turns up stale references. (follow-up to #249; cross-references shared/schemas/src/linkml/geojson.yaml post-#249) | - | - | - | - | Low | approved | | 2026-05-19 | 2026-05-31 | +| ~~266~~ | ~~Tech Debt~~ | ~~Spatial-shape cleanup follow-up to #249 — #249/spec-261 unifies the LinkML schema's spatial-viewport representation onto `ViewportPolygon`, removing the parallel `bbox: float[4]` + `zoom: float` + `center: float[2]` fields from `SystemStateProperties` (per Article XIV.1 — verified zero runtime blast radius today). If any docs, ADRs, ARCHITECTURE.md sections, code comments, or external references still mention the `bbox`/`center` shape after #249 ships, they need updating. Likely affected: `docs/project_notes/decisions.md` (any ADR referencing the spatial SystemState shape), `ARCHITECTURE.md` schema-extension passages, MCP envelope docs if any, and any spec for #215 / #237 that referenced the bbox shape. Estimate 0.5–1 dev-day. Trigger: #249 has shipped and a grep for `SystemStateProperties.*bbox` / `state_type.*spatial.*bbox` over the repo turns up stale references. (follow-up to #249; cross-references shared/schemas/src/linkml/geojson.yaml post-#249)~~ | ~~-~~ | ~~-~~ | ~~-~~ | ~~-~~ | ~~Low~~ | ~~complete~~ | | ~~2026-05-19~~ | ~~2026-06-01~~ | | 265 | Research Spike | [MP4 / GIF export of a Storyboard traversal — encoding-pipeline trade-off spike](https://github.com/debrief/debrief-future/issues/632) — time-boxed (1–2 days) research evaluating at least three encoding-pipeline candidates: (1) client-side `MediaRecorder` + `ffmpeg.wasm` (~20MB bundle bloat, fully offline, ships inside the briefing zip too), (2) service-side Python `services/export/` + system `ffmpeg` (clean Article IV.1, faster, but new system dep + doesn't ship inside the air-gapped briefing zip), (3) hybrid using WebCodecs API with `ffmpeg.wasm` GIF fallback. Deliverable = `docs/project_notes/storyboard-video-export-research.md` with bundle-size measurements, encoding-time benchmarks against a 10-Scene 1080p Storyboard, output-quality samples under `evidence/`, Article I compatibility analysis, briefing-zip (#264) integration story per option, recommendation + a follow-up implementation backlog item. **No shipping code from this ticket.** (split from #229 via `/interview 229`; independent of #263/#264 but recommendation should consider both) | 2 | 2 | 4 | 8 | Low | approved | E13 | 2026-05-19 | 2026-05-19 | | 235 | Research Spike | [[E13] NL Co-pilot for the open plot — feasibility spike](docs/ideas/235-nl-copilot-feasibility-spike.md) — time-boxed (≤3 days) scoping research before committing to a multi-item epic. Tests whether NL commands acting on the **open plot** (filter / hide / run-tool / summarise), surfaced via a CoPilot panel or command pane **inside Debrief**, can be powered today by a cloud LLM (#191's Anthropic pipeline) with a clear upgrade path to a **local LLM at delivery time** (Article I, defence context — no internet). Successor to rejected #195. (requires #191 NL pipeline pattern; reuses debrief-calc tool execution) | - | - | - | - | Low | approved | E13 | 2025-01-01 | 2026-05-31 | | ~~236~~ | ~~Infrastructure~~ | ~~[Web-shell STAC write path — IndexedDB browser persistence + unified `StacWriter` interface across both hosts](specs/236-web-shell-stac-writes/spec.md) — replace `apps/web-shell/src/mocks/stacService.ts`'s read-only mock with a real STAC write layer (Storyboard captures, metadata patches, GeoJSON writes, new STAC item creation) so web-shell persists captures across reloads. Phase 1 (this spec, 5–8 dev-days): IndexedDB-only browser persistence (web-shell stays a pure static site, no Node backend); both hosts implement a shared `StacWriter` TypeScript interface — VS Code's adaptor wraps existing Node-fs code, web-shell's adaptor is new IndexedDB-backed. Bundled sample catalog stays read-only; user writes layer on top as overlays. **Constitution amendment required** — Article IV.4 'persistence-host abstraction', formalising #174's host-adaptor pattern + machine-enforced via ESLint (FR-025). Last-write-wins conflict model (mirrors current VS Code behaviour). Catalog zip export deferred to a separate spec. (closes the FR-WEB-029a session-only badge from #235; depends on the #174 host-adaptor pattern; extends #193's `updateItemMetadata`)~~ | ~~4~~ | ~~3~~ | ~~3~~ | ~~10~~ | ~~Medium~~ | ~~complete~~ | | ~~2025-01-01~~ | ~~2026-05-01~~ | diff --git a/docs/project_notes/issues.md b/docs/project_notes/issues.md index 1a09cfbe9..39622071d 100644 --- a/docs/project_notes/issues.md +++ b/docs/project_notes/issues.md @@ -109,3 +109,8 @@ Each entry should include: - **ADR**: ADR-034 / ADR-035 / ADR-036 in `docs/project_notes/decisions.md` - **Evidence**: `specs/261-session-state-systemstate/evidence/` - **Follow-ups**: #250 (web-shell durable auto-commit-trigger UX — the FR-009a in-memory mirror lands here, IDB durability is the residual); #266 (purge stale bbox/center references); #267 (out-of-window current_time policy); #268 (broader multi-asset save atomicity); tasks T052–T054 (active_storyboard interactive path kept on @debrief/components per R-011, documented in ADR-036). + +### 2026-06-01 - #266: Purge stale `bbox`/`zoom`/`center` SystemState-spatial references (spec-261 follow-up) +- **Status**: Implementation complete (PR pending) +- **Description**: Documentation-hygiene follow-up to #249/spec-261, which unified the `SystemStateProperties.spatial` shape onto `viewport: ViewportPolygon` (+ optional `rotation`), removing the legacy `bbox: float[4]` / `zoom: float` / `center: float[2]` fields. A repo-wide sweep found stale prose still describing the old shape in one forward-facing doc and two shipped specs; all were updated to the `ViewportPolygon` shape: `shared/schemas/README.md` (state.spatial variant line); `specs/237-active-storyboard-persistence/data-model.md` (SystemStateTypeEnum.spatial description); and `specs/022-system-kind-discriminator/` in full — the spec that originally *defined* the bbox/zoom/center shape — covering its acceptance criterion, Key-Entities prose, research rationale, data-model field table + validation rule, the Python usage example, and `evidence/sample-spatial.json` (rewritten to the 4-corner polygon and re-validated against the live `SystemState` Pydantic model). A "superseded by #261" note was added atop spec-022. Verified clean: the trigger greps (`SystemStateProperties.*bbox`, `state_type.*spatial.*bbox`, `(bbox, zoom[, center])`) return zero SystemState hits. Deliberately retained (different, current concepts): the storyboard `Viewport` type (`center`/`zoom`/`bearing`), STAC `spatial.bbox` collection extent, standard GeoJSON `bbox` members, and the new `ViewportPolygon.zoom` slot. No code, schema, or fixture changes — prose + one evidence fixture only. +- **Backlog**: BACKLOG.md item #266 (marked complete) diff --git a/shared/schemas/README.md b/shared/schemas/README.md index 9f0e57967..ef1b580c4 100644 --- a/shared/schemas/README.md +++ b/shared/schemas/README.md @@ -35,7 +35,7 @@ SystemState features store application state alongside spatial data using Point Three state variants are supported: - `state.temporal` - Time viewport (start/end times) -- `state.spatial` - Map viewport (bbox, zoom, center) +- `state.spatial` - Map viewport (ViewportPolygon + rotation) - `state.selection` - Selected feature IDs ## Generated Artifacts diff --git a/specs/022-system-kind-discriminator/data-model.md b/specs/022-system-kind-discriminator/data-model.md index 5a8d663e0..707ee2e6f 100644 --- a/specs/022-system-kind-discriminator/data-model.md +++ b/specs/022-system-kind-discriminator/data-model.md @@ -82,9 +82,8 @@ Extends SystemStateProperties for spatial viewport state. |-------|------|----------|-------------| | `kind` | FeatureKindEnum | Yes | "SYSTEM" | | `state_type` | SystemStateTypeEnum | Yes | "spatial" | -| `bbox` | float[4] | Yes | [minLon, minLat, maxLon, maxLat] | -| `zoom` | float | No | Map zoom level | -| `center` | float[2] | No | [lon, lat] center point | +| `viewport` | ViewportPolygon | Yes | 4-corner polygon [NW, NE, SE, SW], each `{longitude, latitude}`, with optional `zoom` | +| `rotation` | float | No | Map rotation in degrees (0-360) | ### SelectionStateProperties (NEW) @@ -114,7 +113,7 @@ GeoJSON Feature for system state storage. 3. **Kind Consistency**: `properties.kind` must equal `"SYSTEM"` 4. **State Type Validity**: `properties.state_type` must be valid enum value 5. **Temporal Range**: `start_time` must be before or equal to `end_time` -6. **Bbox Validity**: `bbox` must have exactly 4 values in valid coordinate ranges +6. **Viewport Validity**: `viewport.coordinates` must contain exactly 4 corners, each within valid coordinate ranges ## State Transitions diff --git a/specs/022-system-kind-discriminator/evidence/sample-spatial.json b/specs/022-system-kind-discriminator/evidence/sample-spatial.json index 51f42381a..2e9a1e2b0 100644 --- a/specs/022-system-kind-discriminator/evidence/sample-spatial.json +++ b/specs/022-system-kind-discriminator/evidence/sample-spatial.json @@ -8,8 +8,15 @@ "properties": { "kind": "SYSTEM", "state_type": "spatial", - "bbox": [-5.5, 49.5, 1.5, 55.0], - "zoom": 8.5, - "center": [-2.0, 52.25] + "viewport": { + "coordinates": [ + { "longitude": -5.5, "latitude": 55.0 }, + { "longitude": 1.5, "latitude": 55.0 }, + { "longitude": 1.5, "latitude": 49.5 }, + { "longitude": -5.5, "latitude": 49.5 } + ], + "zoom": 8.5 + }, + "rotation": 0 } } diff --git a/specs/022-system-kind-discriminator/evidence/usage-example.md b/specs/022-system-kind-discriminator/evidence/usage-example.md index 23c1d2679..01783c1de 100644 --- a/specs/022-system-kind-discriminator/evidence/usage-example.md +++ b/specs/022-system-kind-discriminator/evidence/usage-example.md @@ -3,7 +3,7 @@ ## Python (Pydantic) ```python -from debrief_schemas import SystemState, SystemStateProperties, GeoJSONEmptyPoint +from debrief_schemas import SystemState, SystemStateProperties, GeoJSONEmptyPoint, ViewportPolygon, Coordinate # Create temporal viewport state temporal_state = SystemState( @@ -26,9 +26,16 @@ spatial_state = SystemState( properties=SystemStateProperties( kind="SYSTEM", state_type="spatial", - bbox=[-5.5, 49.5, 1.5, 55.0], - zoom=8.5, - center=[-2.0, 52.25] + viewport=ViewportPolygon( + coordinates=[ + Coordinate(longitude=-5.5, latitude=55.0), + Coordinate(longitude=1.5, latitude=55.0), + Coordinate(longitude=1.5, latitude=49.5), + Coordinate(longitude=-5.5, latitude=49.5), + ], + zoom=8.5, + ), + rotation=0 ) ) diff --git a/specs/022-system-kind-discriminator/research.md b/specs/022-system-kind-discriminator/research.md index 069fa74e6..082498c85 100644 --- a/specs/022-system-kind-discriminator/research.md +++ b/specs/022-system-kind-discriminator/research.md @@ -39,7 +39,7 @@ **Rationale**: - `state.temporal`: `start_time` (datetime), `end_time` (datetime) -- `state.spatial`: `bbox` (array of 4 floats), `zoom` (optional float) +- `state.spatial`: `viewport` (`ViewportPolygon` — 4 corners + optional `zoom`), `rotation` (optional float) - `state.selection`: `selected_ids` (array of strings) - Using `state_type` discriminator within properties enables type-safe access - Follows same pattern as existing kind discriminator for feature types diff --git a/specs/022-system-kind-discriminator/spec.md b/specs/022-system-kind-discriminator/spec.md index e71db6f23..daca8d164 100644 --- a/specs/022-system-kind-discriminator/spec.md +++ b/specs/022-system-kind-discriminator/spec.md @@ -5,6 +5,8 @@ **Status**: Draft **Input**: Add SYSTEM kind discriminator for storing non-spatial system state (viewports, selections) as GeoJSON Features +> **Superseded shape (spec-261 / #249):** The `state.spatial` viewport originally defined here as `bbox`/`zoom`/`center` was later unified onto a single `viewport: ViewportPolygon` (+ optional `rotation`). The spatial examples in this spec have been updated to reflect that current shape; see `specs/261-session-state-systemstate/`. + ## User Scenarios & Testing *(mandatory)* ### User Story 1 - Save Plot with Viewport State (Priority: P1) @@ -17,7 +19,7 @@ An analyst working on a complex plot adjusts the map view to focus on a specific **Acceptance Scenarios**: -1. **Given** a plot with the map zoomed to a specific area, **When** the user saves the plot, **Then** the spatial viewport (bbox, zoom) is stored as a SYSTEM feature with id `state.spatial` +1. **Given** a plot with the map zoomed to a specific area, **When** the user saves the plot, **Then** the spatial viewport (ViewportPolygon) is stored as a SYSTEM feature with id `state.spatial` 2. **Given** a plot with a time range filter applied, **When** the user saves the plot, **Then** the temporal viewport (start/end times) is stored as a SYSTEM feature with id `state.temporal` 3. **Given** a saved plot with viewport state, **When** the user opens the plot, **Then** the application can retrieve the SYSTEM features by their known IDs @@ -78,7 +80,7 @@ A developer integrating with Debrief's data model needs to create or validate SY - **SYSTEM Feature**: A GeoJSON Feature with `kind: "SYSTEM"` storing non-spatial application state. Distinguished by null geometry and reserved `state.*` feature IDs. - **Temporal Viewport** (`state.temporal`): Stores the visible time range with `start` and `end` ISO8601 timestamps in properties. -- **Spatial Viewport** (`state.spatial`): Stores the visible map extent with `bbox` (array of 4 coordinates) and optional `zoom` level in properties. +- **Spatial Viewport** (`state.spatial`): Stores the visible map extent as a `viewport` (a 4-corner `ViewportPolygon` carrying an optional `zoom`) plus optional `rotation` in properties. - **Selection State** (`state.selection`): Stores an array of selected feature IDs in properties. ## Success Criteria *(mandatory)* diff --git a/specs/237-active-storyboard-persistence/data-model.md b/specs/237-active-storyboard-persistence/data-model.md index 30e7ccc81..7412c8829 100644 --- a/specs/237-active-storyboard-persistence/data-model.md +++ b/specs/237-active-storyboard-persistence/data-model.md @@ -34,7 +34,7 @@ permitted `state_type` value. temporal: description: Time viewport state (start/end times) spatial: - description: Map viewport state (bbox, zoom) + description: Map viewport state (ViewportPolygon) selection: description: Feature selection state (selected IDs) active_storyboard: # ← NEW