Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .agents/skills/domain/batch-lifecycle/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
name: batch-lifecycle
description: The settled domain model for batch, job, and asset-progress lifecycles in VisionSet. Consult before any change that reads or writes batch state, job state, asset progress, promotion, or schema pinning — in any layer. These decisions are settled; do not re-litigate them in implementation tasks.
---

# Batch lifecycle (settled model)

## State machines — single source of truth

The kernel tables are authoritative. Quote them; never re-derive them.

- `BATCH_TRANSITIONS` (`kernel/domain/batch.py`): `draft → approved → in_annotation → completed`. **One-way. `completed` has no exit.**
- `JOB_TRANSITIONS` (`kernel/domain/task.py`): `pending → in_progress → completed`.
- `ASSET_PROGRESS_TRANSITIONS` (`kernel/domain/task.py`): `unannotated ↔ annotated`, `annotated|unannotated → skipped → unannotated`, `annotated → review_pending → annotated|accepted`, `accepted` terminal.
- Derived sets: `SETTLED_PROGRESS = {annotated, skipped, accepted}` (doesn't block completion), `PROMOTABLE_PROGRESS = {annotated, accepted}` (`skipped` never promotes).

**All legality checks go through the `require_move` funnel** (`domain/transitions.py`) or a named set consulted beside it. Hand-rolled membership checks outside the funnel are forbidden (the `repin` hand-roll was finding F13 and has been/is being folded in).

## Settled decisions

1. **Forward-only correction; no reopen.** A `completed` batch is immutable as a workflow unit. There is no `completed → *` transition and none will be added. Corrections happen through a **correction batch**: a new batch over a chosen asset set, pinning the active schema at its own approval, carrying lineage to its parent. Decisions #301/#303 ("settled batches re-enterable to edit") are **superseded** — the legitimate intent behind them ("add one more box later") is served by correction batches. UI on a `completed` batch offers view-only entry plus "Create correction batch" (once it exists), never editing.
2. **Annotation writes are gated on progress** (F11, accepted 2026-08): the kernel refuses annotation add/update/delete unless the asset's progress is `unannotated` or `annotated`. Correcting a `skipped`/`review_pending`/`accepted` asset means moving its progress first (where legal) or a correction batch. Silent label-drop at promotion must be impossible.
3. **`completed` batches cannot be deleted** (F12, accepted 2026-08): `BatchService.delete` refuses `completed` regardless of `confirm`. History is not disposable.
4. **Review is a product flow, not an API-only edge** (F24, decided 2026-08): the annotator provides `annotated → review_pending` (submit for review) and the review-side moves (`→ annotated` reject, `→ accepted`). The gallery's "In review" grouping is backed by reachable UI.
5. **Promotion is not a transition.** It is idempotent trunk-union from a `completed` batch; batch state does not change. Its result must be observable (promoted count, trunk membership on a read model) — invisible success is a bug, not a design.
6. **Schema pinning**: pinned at approval; movable only via `repin` while `approved | in_annotation`; frozen at `completed`. A schema-publish + repin chain on a batch where repin is illegal must not half-apply (F23) — check repin legality (capabilities) before publishing.
7. **Immutability hierarchy**: releases are content-immutable (the hash is the contract) > `completed` batches are workflow-immutable > everything else is mutable. Do not promote anything else to immutable "for safety".

## What is NOT settled (do not improvise)

- Trunk supersession semantics when a correction batch re-annotates an already-promoted asset (audit G5) — requires an explicit design decision before implementation.
- Cross-batch progress reconciliation for an asset in multiple batches (F14).

If a task seems to need either, stop and flag it instead of choosing a policy inline.
41 changes: 41 additions & 0 deletions .agents/skills/frontend/information-architecture/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
name: information-architecture
description: The canonical sitemap and navigation rules for the VisionSet app. Consult before adding, moving, or removing any route, tab, screen, nav entry, or cross-screen link. Any change to where something lives requires updating the sitemap in this file in the same PR.
---

# Information architecture

## Canonical sitemap

Navigation maps 1:1 to domain objects. This is the target structure; if implementation differs, implementation is what's wrong.

```
/projects Projects list
/projects/:id Project — tabs:
?tab=overview Overview (dashboard, see below)
?tab=batches Batches (workflow)
?tab=dataset Dataset (trunk + releases) ← primary tab, not a buried route
?tab=schema Schema (contract)
└─ version history: subsection INSIDE Schema, not a sibling tab
/projects/:id/ingest Ingest flow
/projects/:id/batches/:batchId Batch workspace (gallery)
/jobs/:jobId Annotator (full-bleed)
```

Rules derived from the 2026-08 audit (§6):

- **Dataset is first-class.** It is the product's central object and must be reachable in ≤1 click from any project tab. It is never gated behind, or discoverable only through, onboarding UI. Promotion success links onward to it; the gallery links to it once a batch is `completed`.
- **"Schema history" is not a sibling tab.** Version history lives inside the Schema tab (the `VersionNavigator` seam at `SchemaEditor.tsx:305-311` already exists). The `?tab=versions` value may remain as a redirect for compatibility; it does not appear in the tab bar.
- **The 4-step checklist is onboarding, not navigation.** It renders only in empty/early states, is dismissible, and disappears permanently once the project has a release (`hasReleases` — wire support required, see F17-adjacent work). It never gates anything and is never the sole path to a screen.
- **Overview is a dashboard**: pipeline state of batches, trunk size, latest release, active schema version — each card links to its tab. Overview never duplicates a tab's full function.

## Structural invariants

- **Single route definition site**: `frontend/app/src/routes.tsx`. No routes defined elsewhere.
- **`ui-core` stays router-free.** Screens receive navigation as callback props (`routes.tsx:113-121` pattern). Never import a router in `ui-core`.
- **Back-links are declared** in the routes parent map (`routes.tsx:150-154`) and must point to the contextual parent: the gallery's back is the Batches tab; the Dataset screen's back is the tab the user came from or Overview — never a surprising sibling.
- Tab state lives in `?tab=` with `replace: true`; unknown values fall back to `overview` silently.

## Process rule

Any PR that moves a screen, adds/removes a tab or nav entry, or changes an entry point **must update the sitemap block above in the same PR**, with one line in the PR body: what moved and why. If the sitemap and the change disagree and the sitemap is not updated, the change is wrong by definition.
29 changes: 29 additions & 0 deletions .agents/skills/frontend/ui-capabilities/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
name: ui-capabilities
description: Rules for how the VisionSet frontend decides which actions to offer and how it handles mutation outcomes. Consult before touching any component that renders a state-gated action, any mutation hook, or any error/success feedback. Enforces the capabilities contract and bans the hand-mirrored-table and swallowed-error antipatterns that caused findings F1–F10 of the 2026-08 audit.
---

# UI capabilities

## The one rule

**The frontend never decides what is legal. It renders what the wire declares.** Action availability comes from `allowed_actions` on the resource's wire model (`BatchOut`, `JobOut`, `BatchAssetOut`). The client may cache, group, and label capabilities; it may not compute them.

## Banned patterns (each caused a shipped blocker)

1. **Hand-mirroring kernel transition tables.** `batchState.ts:226` literally documented itself as "a mirror of two rows of the kernel's `ASSET_PROGRESS_TRANSITIONS`" — and the mirror drifted by omitting the batch-state dimension, producing F1/F2. Do not write `canX(state)` helpers that re-derive legality from resource fields. If a capability is missing from the wire, the fix is in the wire/kernel projection, never a client-side workaround.
2. **Swallowed refusals.** No empty `catch {}` around mutation calls (the `queries.ts:889` pattern destroyed every per-frame refusal reason). No mutation whose `isError`/`error` is never rendered (F3, F4, F8, F9). No `void someAsyncMutation()` without a rejection handler (F7).
3. **Invisible success.** If a mutation's response carries meaningful data (e.g. promote returns the assets actually promoted), render it. A label flip is not feedback (F5). Idempotent operations must distinguish "did N" from "nothing to do".
4. **Raw refusal codes as UI.** Refusals render through the shared code→prose vocabulary (one map, product-wide — F16). A bare `BATCH_NOT_IN_ANNOTATION` badge is not a message.

## Required patterns

- **Disabled-with-reason over hidden** for actions absent from `allowed_actions` but meaningful in context: render disabled with a tooltip stating why ("Batch is completed — create a correction batch to edit"). Fully hide only actions that are never meaningful on that screen.
- **Read-only is a mode, not an accident.** Any surface that can open in a state where writes are not permitted (annotator on a non-`in_annotation` batch) must render an explicit read-only mode: visible banner, editing tools disabled, no dirty state possible. "Open and let saves fail" is forbidden.
- **Every mutation call site** answers three questions in code review: where does a refusal render? where does success render? what happens to the rejected promise? If any answer is "nowhere", the change is incomplete.
- The app-level error boundary and `unhandledrejection` handler are load-bearing; never remove or bypass them.

## Scope limits (do not overreach)

- This skill governs *gating and feedback*, not visual design.
- Do not add client-side pre-validation that duplicates kernel checks "for snappiness" — that recreates the mirror. Optimistic UI is allowed only for operations the wire declares and only with rollback + refusal rendering.
49 changes: 49 additions & 0 deletions .agents/skills/process/refactor-protocol/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
name: refactor-protocol
description: Execution rules for any refactoring or feature task in the VisionSet repo — worktree isolation, scope discipline, testing requirements, PR/CI automation, and cleanup. Consult at the start of every implementation task.
---

# Refactor protocol

## Scope discipline

- **The task prompt wins** over issue text, code comments, and your own judgment about "obvious" adjacent improvements. Where prompt and issue conflict, follow the prompt and note the conflict in the PR body.
- **Do not implement open issues "in passing"**, even when the code you're touching invites it. Reference them (`cf. #NNN`) and move on.
- **Do not fix unrelated bugs you discover.** Record them in the PR body under "Found, not fixed".
- **Layer boundaries**: no kernel changes unless the task explicitly grants them; `@visionset/annotator` internals untouched unless named; import-linter contracts must stay green — kernel purity is non-negotiable.
- Settled domain decisions (see `batch-lifecycle` skill) are not re-litigated in implementation. If a task appears to require violating one, stop and flag.

## Worktree isolation

```bash
git fetch origin
git worktree add ../visionset-<task-slug> -b <type>/<task-slug> origin/main
```

All work in the worktree; never the primary checkout. Conventional commits in logical units; wire/kernel changes in their own commits, separate from UI commits.

## Testing requirements

- **Layout, virtualization, and observer behavior are asserted in real chromium (Playwright), never jsdom.** A never-attached ResizeObserver passes green in jsdom forever (bug #159). Column counts, scroll-parent assertions, and re-flow on resize are e2e concerns.
- **Gating changes are tested against the state matrix**: for each affected action, at least one test per relevant resource state proving offered ↔ legal (the capability contract makes this mechanical: declared action succeeds, undeclared action is not offered).
- **Every mutation touched must have a refusal-rendering test**: force the refusal, assert the user sees prose (not a raw code, not nothing).
- E2e fixtures seed all five asset-progress states and at least one batch per batch state when the task touches state-dependent UI.
- Run the full existing suites (Python + TS) and linters; fix what your change broke, and only that.

## PR & CI

1. `gh pr create` — body includes: what changed, "Found, not fixed" list, test plan, `Closes #NNN` only for issues actually and fully closed.
2. `gh pr merge --auto --squash`.
3. Monitor `gh pr checks --watch`; on failure read logs, fix, push. **After 3 consecutive failures of the same check with no clear fix, stop and report** — never loop indefinitely, never disable or skip a failing check to get green.

## Cleanup

After merge confirmation (`gh pr view --json state,mergedAt`):

```bash
git worktree remove ../visionset-<task-slug>
git branch -d <type>/<task-slug>
git fetch --prune
```

If not merged at session end: leave the worktree, report path + branch + PR URL + CI status.
8 changes: 8 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ re-run.
| `annotator-core` | Headless annotator boundary: pure TS core, React only in adapters | `.agents/skills/frontend/annotator-core/SKILL.md` |
| `nodejs-setup` | Node 24, pnpm workspace, filters, workspace deps | `.agents/skills/frontend/nodejs-setup/SKILL.md` |
| `docker-dev` | Dev-only compose environment, profiles, logs | `.agents/skills/infra/docker-dev/SKILL.md` |
| `batch-lifecycle` | Settled batch/job/asset-progress model — consult in **any** layer before touching state | `.agents/skills/domain/batch-lifecycle/SKILL.md` |
| `ui-capabilities` | How the frontend decides what to offer, and how refusals surface | `.agents/skills/frontend/ui-capabilities/SKILL.md` |
| `information-architecture` | The canonical sitemap: routes, tabs, entry points, back-links | `.agents/skills/frontend/information-architecture/SKILL.md` |
| `refactor-protocol` | Execution rules for any implementation task: worktree, scope, tests, PR/CI | `.agents/skills/process/refactor-protocol/SKILL.md` |

### Auto-invoke

Expand All @@ -48,6 +52,10 @@ Read the skill **before** writing code in that area.
| Annotation/canvas interaction, geometry, undo/redo, render adapters | `annotator-core` |
| Installing packages or running frontend scripts | `nodejs-setup` |
| Starting or debugging Docker | `docker-dev` |
| Starting **any** implementation task, before the first edit | `refactor-protocol` |
| Reading or writing batch state, job state, asset progress, promotion, schema pinning — in any layer | `batch-lifecycle` |
| Rendering a state-gated action, a mutation hook, or error/success feedback | `ui-capabilities` |
| Adding, moving, or removing a route, tab, screen, nav entry, or cross-screen link | `information-architecture` |

## Project overview

Expand Down
34 changes: 34 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,40 @@ the 409 is resending the *identical* request with one extra parameter. The route
pre-check either one: the flag goes to the SDK and the SDK's refusal is what carries
`CONFIRMATION_REQUIRED` or `DESTRUCTIVE_SCHEMA_CHANGE`.

**A stateful resource declares what it allows.** `BatchOut`, `JobOut` and `BatchAssetOut` each
carry `allowed_actions` — a list of names, closed and published in the spec as `BatchAction`,
`JobAction` and `AssetAction`.

```
GET /batches/{id} → { "state": "in_annotation",
"allowed_actions": ["complete", "repin", "delete"], … }
```

**A client renders these; it never computes them.** Re-deriving the rules from `state` and
`progress` is what the browser used to do, and its copy drifted by dropping the batch-state
dimension — which is why skipping a frame was offered on a batch the kernel refused every write
into. If an action you need is missing, the fix is in the projection, never a second copy of the
rule.

The declarations come from `kernel/domain/capabilities.py`, which reads the same transition
tables and named sets the services enforce with;
`tests/kernel/test_capabilities.py` drives both halves over a real workspace for every reachable
state, so the two cannot move apart.

Two of them are worth reading precisely:

- **`complete` on a batch is declared from the transition table alone**, so it can still answer
409 `BATCH_NOT_COMPLETE`: completion is derived from the jobs, which is a read the projection
does not make. Read it as *this batch is at the point where completing is the next move*. A
job's `complete` carries no such caveat — a job ships its own per-asset tally, so the
`SETTLED_PROGRESS` condition is applied.
- **`annotate` on a batch asset is the right to write labels**, not a progress move: it is
declared exactly when `POST /jobs/{id}/annotations` will be accepted. An annotator deciding
whether to open in edit mode should read this rather than infer it.

An empty list is normal and means what it says. Every asset of a `completed` batch declares
nothing, because nothing may be written into a batch that has closed.

**Statuses.** 201 with the created resource in the body; 200 for a read or an update; 204 with an
empty body for a delete. And **202 when the work has not happened yet** — see below.

Expand Down
Loading
Loading