0.1b — validate lab.toml on extension load with field-precise errors (closes #16) - #29
Conversation
jack-champagne
left a comment
There was a problem hiding this comment.
Code's correct — reproduced all 7 ACs against real ajv output; the load-time check delegates to the single @amicode/schema validator and is non-fatal. Approving the code.
Two nits that live in #28's schema (flagging here since this is the PR that owns the "no silent wrong-hardware" story): delta_GHz has no range bound while every other [transmon] field does — a sign-flipped or garbage anharmonicity passes validation, the exact class this slice exists to catch; and the only valid fixture is demo-lab, no Schuster profile (the PRD's demo forcing-function).
Tests + Phase 1: the negative matrix is partial vs #16 — it hits levels / drive_max / schema_version / unknown-key but not the range bounds (omega_GHz ≤ 100, drive_max_GHz ≤ 10) or lab.name minLength, and the "parity over the shared negative-fixture corpus" AC is exercised on a single input (inline). Two Phase-1 prerequisites this surfaces: (1) there is no test harness for the VS Code host layer — the on-load toast/channel path is untested, and Phase 1 stacks more UI (inspector multi-run, catalog tree) on that same seam, so an extension-host integration approach should land before it compounds; (2) lab.toml has no provides block yet — the cloud solve-service (aws-infra) keys on lab provides to reproduce the runtime and validate requires ⊆ provides, so that dimension + its validation is a cloud/Phase-1 prerequisite, not optional polish.
Sequencing, not a change: stacked on #28 (CHANGES_REQUESTED), and the tests hardcode #28's error strings — rebase + re-run after #28 settles, and don't merge ahead of it.
| if (lab.state === "invalid") { | ||
| runsChannel.appendLine(`[lab] ${lab.path} is INVALID:`); | ||
| for (const e of lab.errors) runsChannel.appendLine(` ${e}`); | ||
| void vscode.window.showErrorMessage( |
There was a problem hiding this comment.
[nit] this showErrorMessage + output-channel path is the one surface the unit tests can't reach — no VS Code host in CI, so checkLabToml is covered but the toast/channel wiring isn't. I'll verify it by hand on the packaged VSIX (bad ~/.amico/lab.toml → reload → toast + "Amicode — runs" list). Non-blocking — but it's the seam Phase 1's UI work compounds on, so it wants a host-level test approach soon.
There was a problem hiding this comment.
Acknowledged — the toast/channel wiring stays unit-untestable (no VS Code host in CI). Noted as a Phase-1 prerequisite (a host-level/integration-test approach before the inspector/catalog UI compounds on this seam); not adding host-test infra in this slice. checkLabToml itself is covered.
| it("unrecognized schema_version → version-specific error", () => | ||
| expect(has(errs(VALID.replace('schema_version = "1"', 'schema_version = "9"')), "/schema_version: unrecognized version")).toBe(true)); | ||
|
|
||
| it("parity: checkLabToml uses the SAME validator as @amicode/schema directly (no second path)", () => { |
There was a problem hiding this comment.
[test] this proves parity on one input; #16's AC says "over the shared negative-fixture corpus." Delegation makes drift structurally impossible so it's not urgent — but loop the corpus to meet the AC literally, and add the range-bound cases the matrix above skips (omega_GHz ≤ 100, drive_max_GHz ≤ 10, lab.name minLength).
There was a problem hiding this comment.
Done — the parity test now loops a corpus (valid + missing/wrong-type/out-of-range/unknown-key/absent+unrecognized-version), asserting checkLabToml === validateFile on each, and the range-bound cases (omega_GHz≤100, drive_max_GHz≤10, the now-bounded delta_GHz, lab.name minLength) are added. Plus a Schuster-profile valid fixture (negative-δ, 4 levels).
…errors (closes #16) A partner lab's lab.toml is the only place hardware params enter a solve; β had no validation, so a malformed/mistyped config silently solved against the wrong hardware or failed opaquely mid-solve. This validates it on extension load against the shared @amicode/schema lab schema (defined in 0.1a) and surfaces a field-precise error (offending key + dotted path), non-fatally. - src/lab_config.ts: resolveLabTomlPath (default ~/.amico/lab.toml, ~ expansion) + checkLabToml → absent | valid | invalid{errors}, via the SINGLE @amicode/schema validator (no second validation path). - extension.ts activate(): validates on load; invalid → showErrorMessage with the first field-precise error + full list in the "Amicode — runs" output channel. - amicode.labToml config setting (path; empty → ~/.amico/lab.toml). - lab.toml.example: stamped schema_version = "1" so the shipped starter validates. - Tests (lab_config.test.ts, 13): field-precise negative matrix (missing / wrong-type / out-of-range / unknown-key / absent+unrecognized version), parity with @amicode/schema.validateFile (single-path proof), shipped-example conforms. Extension 63 tests green; typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h/Schuster (rebased on #28 rename) Address Jack's #29 nits: - Loop the parity assertion over a CORPUS (valid + missing/wrong-type/out-of-range/ unknown-key/absent+unrecognized-version) asserting checkLabToml === @amicode/schema validateFile on each — was a single input (#16 "over the corpus"). - Add the range-bound negatives the matrix skipped: omega_GHz≤100, drive_max_GHz≤10, delta_GHz (now bounded in #28), and lab.name minLength — each field-precise. - Add a Schuster-profile valid fixture (negative-δ convention, 4 levels) alongside demo-lab — the PRD demo forcing-function + second real-shaped profile. Rebased onto #28's manifest.toml→run.toml rename (lab files don't reference the run-dir header, so clean). extension 64 (+1 packaging skip) green. Non-blocking (Jack): the toast/channel seam has no VS Code host test (no host in CI) and lab.toml has no `provides` — both are Phase-1 prerequisites (provides ties to the 0.1a-follow env-contract), noted for that work, not this slice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5def952 to
ade5a6d
Compare
Stacked on #28 (0.1a). Do not merge.
A partner lab's
lab.tomlis the only place hardware params enter a solve; β had no validation, so a malformed/mistyped config silently solved against the wrong hardware or failed opaquely mid-solve. This validates it on extension load against the shared@amicode/schemalab schema (defined in 0.1a) and surfaces a field-precise error (offending key + dotted path), non-fatally.What's here
src/lab_config.ts:resolveLabTomlPath(default~/.amico/lab.toml,~expansion) +checkLabToml→absent | valid | invalid{errors}, via the single@amicode/schemavalidator (no second validation path).extension.tsactivate(): validates on load; invalid →showErrorMessagewith the first field-precise error + the full list in the "Amicode — runs" channel. Non-fatal — the rest of the extension still activates.amicode.labTomlconfig (path; empty →~/.amico/lab.toml).lab.toml.example: stampedschema_version = "1"so the shipped starter validates.Tests (
lab_config.test.ts, 13)Field-precise negative matrix (missing / wrong-type / out-of-range / unknown-key / absent+unrecognized version), parity with
@amicode/schema.validateFile(asserts the fullerrorsarray — proves single-path), and shipped-lab.toml.exampleconformance. Extension 63 green; typecheck clean.Review
Adversarial code review: no must-fix / should-fix. Verified empirically that
validateFileis throw-safe on missing/garbage/directory inputs (so the load seam can't break activation), and thatlab.schema.jsonexactly matches the shippedlab.toml.example(a real install won't red on load).🤖 Generated with Claude Code