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
2 changes: 2 additions & 0 deletions .agents/skills/process/refactor-protocol/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ All work in the worktree; never the primary checkout. Conventional commits in lo
```

The script sets `CI=1` for the Playwright steps itself, so that is no longer yours to remember. **`--fast` is never enough before a push.** The real-server cycle run is mandatory for anything touching state, gating, or progress: it was three separate times the *only* suite to catch a regression — a stale job declaration, a label flip standing in for feedback, and a progress counter running backwards. — 2026-08 run, T3/T5/T6; #314
- **When the machine is saturated, the fallback is declared — never silent.** A green `bash scripts/check.sh` is still what a merge requires. When another session has the box, and you can *show* it — load average, the competing processes, `ps aux | grep` output — the sanctioned substitute is: every static gate (`ruff check .`, `ruff format --check .`, `mypy`, `lint-imports`, the `node --test` script gates), the full frontend build and test suite, and every pytest module the change touches, with **full green CI on clean runners as the arbiter**. That is not a lowering of the bar: a timing-sensitive suite at load average 60 tells you nothing it would not also tell you at load average 6000. **Say so in the PR body before the merge, naming which suites did not run and why.** A merge that lets a reader infer a green local gate that never happened is a protocol violation, not a shortcut — and the fallback is only available for a machine you can evidence, not for one you are impatient with. — #339
- **`CI=1` on any Playwright run you invoke by hand.** `playwright.config.ts` sets `reuseExistingServer: !CI`, so a stale vite server on :5273 answers instead of your build and produces failures that read as code bugs. `check.sh` does this for you; `npx playwright test` typed directly does not. — 2026-08 run, T3
- **To rerun the cycle suite N times, use `--repeat-each=N`** — it costs one build rather than N, because the suite's names are run-scoped since #314. Before that a fixed project name made repeat 2 die on `POST /projects → 409`, and repetition meant N whole invocations at ~90 s of rebuild each.
- **`git add` new files before trusting any local check run.** Several gates read `git ls-files` — the index, not the working tree — so an untracked new file is invisible to them and passes locally while failing in CI. — 2026-08 run, T4
- **After a rebase or merge that brings in commits you did not write, lint the *whole tree*** — `ruff check .`, not the files you touched. A rename is a whole-tree fact: your branch renames a symbol, somebody else's branch adds a *new* use of the old name, and git merges both without a conflict because they are different lines. Re-running the tests you edited proves nothing either when the surviving use is a type annotation, which is never evaluated. #339 renamed a test double; #281 landed mid-flight with a fixture annotated on the old name; every targeted pytest module passed and CI answered `F821`. — #339
- **A test double must not encode invisible-order or frozen-state semantics.** Put defaults in the *unmatched-request fallback* so an explicit stub always wins whichever order it was registered in, and derive stub responses from the state the test walks rather than from frozen literals. Both failure modes make a test assert against the fixture instead of the code, and both are silent. — 2026-08 run, T6/T7/T10

## PR & CI
Expand Down
10 changes: 6 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ nothing was being distributed. This is the first version that is.
### Changed

- **`POST /releases/{id}/export` answers `202` instead of the archive** (#328). **Breaking, for
this one endpoint.** It used to block until the exporter finished; a real format walks every
asset in a release and copies its bytes, which is minutes of work behind a request with no way
to report progress and every proxy's timeout in front of it. It now returns a job to poll and
a `Location`, with the archive at `GET /background-jobs/{id}/artifact`.
this one endpoint.** It used to be a synchronous `FileResponse` that blocked until the exporter
finished; a real format walks every asset in a release and copies its bytes, which is minutes
of work behind a request with no way to report progress and every proxy's timeout in front of
it. It now returns `202` with a job to poll and a `Location: /background-jobs/{job_id}`, and
the archive comes from `GET /background-jobs/{job_id}/artifact` once that job reports
`succeeded`.

Everything a *request* can refuse is still refused on the request: an unknown format is a 404
and an unconsented lossy export a 409, neither creating a job. The browser's consent flow is
Expand Down
33 changes: 33 additions & 0 deletions docs/background-jobs.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,31 @@ here `attempt` on one row would have to mean two different things, and a list of
jobs would hide a history behind a single line. So the orphan sweep re-enqueues
an idempotent job as a fresh one, and a list shows the crash *and* the recovery.

**The `job` table has no `project_id`, and no foreign key at all.** Settled, not
outstanding — it is the thing about this schema most likely to read as an
oversight, so: do not add one.

A job is *workspace-scoped execution plumbing*. What a job is **about** lives in
its `payload`, keyed by id, and different types are about different things: an
export is about a release, an ingest about an ingest job, the next one about
something else again. A `project_id` would be null for some types and wrong for
others, and a column that is sometimes meaningless is one every reader learns to
distrust.

The keyless half is load-bearing. Under `PRAGMA foreign_keys = ON` a key means a
cascade, and a cascade here would delete the record of work that already
*happened*: "this export ran and here is where it put the archive" stays true
after the release is gone, and a job row outliving its subject is the behaviour
rather than a leak. It also leaves this the one table a later migration can widen
freely — a column carrying a foreign key cannot arrive by `ALTER TABLE` in
SQLite, so a table with no keys never needs the rebuild that `_tables.py`
documents for everything else.

What it costs, stated: there is no `GET /projects/{id}/background-jobs` and there
cannot be one without reading payloads. Nobody has asked. If somebody does, the
honest shape is a nullable, unindexed, **no-key** `scope` column written by
whoever enqueues — not a foreign key.

**Cancellation is cooperative.** A queued job is cancelled outright; a running
one is only *told*, and its handler decides where stopping is safe. Nothing is
killed mid-statement, because a handler halfway through writing rows would leave
Expand Down Expand Up @@ -183,6 +208,14 @@ answered them.
- **No scheduled or recurring jobs.** A job exists because a request made one.
- **No hard cancellation.** See above.
- **No cross-process events.** A durable outbox is pro/multi-node territory.
- **No artifact retention policy.** What a job leaves in `<workspace>/exports/`
stays there until somebody deletes it — no TTL, no size cap, no sweeper, and no
`DELETE` route. Deliberate: the disk is the user's, and it is the posture blobs
and staged uploads already have. `docs/releases.md` and `docs/workspaces.md`
argue it; a deployment that wants a policy owns one, over plain files.
- **No `project_id` on the `job` table**, and no foreign key at all. See "The
decisions, and why" above — scoping lives in the payload, and a key would
cascade away the record of work that already happened.
- **`ingest_job` and `job` coexist.** An ingest has two rows for one run: the
`ingest_job` is the domain record and the wire contract, the `job` is execution
plumbing. Collapsing them is a migration with its own wire-contract discussion.
Expand Down
11 changes: 11 additions & 0 deletions docs/releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,17 @@ like `uploads/`, and the handler clears it before each run so the archive descri
and not the last one. The archive is a sibling of that directory rather than a file inside it, so
a re-export cannot sweep the previous one into the new one.

**Nothing expires it, and that is the policy rather than a gap.** An export stays in
`<workspace>/exports/` until somebody deletes it: there is no TTL, no size cap, no sweeper, and
no `DELETE` route. VisionSet is local-first and the disk is the user's — a tool that quietly
removed a training set somebody had exported, on a schedule they did not choose, would be
solving a problem they did not report by taking an action they cannot undo. It is the same
posture blobs and staged uploads already have, and it is stated here rather than left implicit
so that "exports are never cleaned up" reads as an answer instead of an omission. A deployment
that does want a policy owns one: the directory is plain files under a path the operator chose,
and `find`, a cron job or a retention rule on the volume are all better placed to express it
than this product is. Re-exporting is free, so deleting the lot is safe.

**Which formats exist is a property of the deployment**, so `GET /formats` answers it rather
than this document. `lossy` is on the row so a client knows before it POSTs whether the export
will need `allow_lossy=true`, instead of discovering it by getting a 409; `geometries` and
Expand Down
19 changes: 14 additions & 5 deletions docs/workspaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,27 @@ reason: HTTP has bytes where the kernel has paths.
`SourceService` — which registers a source by *path* — has a path to be given.

`exports/<release_id>/<format>/` is where a format plugin writes, and `<format>.zip` beside it
is the archive the export route streams back. The kernel does not choose the location:
`ReleaseService.export` writes into whatever directory it is handed, and this one is the REST
surface's choice because the REST surface is the caller that has to turn the result into a
response. The route clears the directory before each run, which it can do safely precisely
because it built the path out of the workspace root, a release id and a format name.
is the archive `GET /background-jobs/{job_id}/artifact` streams back. The kernel does not choose
the location: `ReleaseService.export` writes into whatever directory it is handed, and this one
is chosen by the export *handler* (`visionset/jobs/export.py`) — since #328 an export runs in a
worker rather than in the request, and the handler is the caller that has to turn the result
into something a later request can serve. It clears the directory before each run, which it can
do safely precisely because it built the path out of the workspace root, a release id and a
format name.

The kernel neither writes nor reads either, `open` simply tolerates them the way it tolerates
anything else beside the database and `blobs/`, and the CLI and MCP surfaces create neither —
they already hold real paths. Like blobs, staged uploads and finished exports are never
deleted: a workspace grows with what was offered to it and with what was asked of it, not only
with what it kept.

**That "never" is a decision, not an unwritten TODO.** There is no TTL, no size cap and no
sweeper for any of the three, and none is planned for the OSS product: the disk belongs to the
user, and a local-first tool that deleted their staged uploads or their exported training set on
a schedule they never chose would be taking an action they cannot undo. What that costs is
honest — a workspace only grows — and the remedy is `rm`, which is safe for `uploads/` and
`exports/` because both are reproducible, and is *not* safe for `blobs/`, which is the data.

The store runs in **WAL mode**, which is why the two sidecars are
part of the format: `close()` checkpoints them into `visionset.db` and removes them, so a
workspace at rest is still just the database and the blobs — but a workspace that is *open*,
Expand Down
Loading