Skip to content

0.1c — amico-validate CLI + CI fast-tier schema gate (closes #17) - #30

Merged
jack-champagne merged 2 commits into
mainfrom
rchari/0.1c-amico-validate-cli
Jun 29, 2026
Merged

0.1c — amico-validate CLI + CI fast-tier schema gate (closes #17)#30
jack-champagne merged 2 commits into
mainfrom
rchari/0.1c-amico-validate-cli

Conversation

@Rchari1

@Rchari1 Rchari1 commented Jun 27, 2026

Copy link
Copy Markdown
Member

Stacked on #29 (0.1b). Do not merge.

A user-facing validator over the shared @amicode/schema validate() core, plus a CI step that reds the PR on a schema-invalid config — no parallel validation logic.

What's here

  • src/cli.ts: amico-validate <file> [--schema <kind>]. File→schema by basename for the fixed-filename artifacts (manifest.toml/result.toml/lab.toml/FINISHED via kindForFilename); --schema override for solvespec/catalog-entry or any non-standard name. Exit 0 valid / 64 invalid-or-usage (Q85). Field-precise stderr (offending key + JSON-pointer path).
  • bin amico-validatedist/amico-validate.js (esbuild, shebang + chmod; ajv bundles).
  • test/fixtures/invalid/: one representative malformed file per kind — the shared negative corpus (the 0.1a valid corpus is the shared positive corpus; one corpus, all surfaces).
  • test/cli.test.ts (8): valid→0 + invalid→64 across every kind, file-role resolution, FINISHED-by-basename, non-filename-needs---schema, field-precise stderr, usage→64, --help→0.
  • ci.yml: fast-tier amico-validate gate over the shipped configs (lab.toml.example + demo manifest/result/FINISHED) after build.

Review

Adversarial code review: no must-fix / should-fix correctness. Verified empirically — every argv/edge path returns the right exit, kindForFilename doesn't misresolve (result.toml.bak → undefined), the CI bash word-split works (local zsh-only quirk), the gate runs after build, shebang + chmod +x present. One optional hardening nit (CI invokes the bundle directly rather than the linked bin) — non-blocking.

Tests

schema 32 / amico-run 47 / extension 63 green; CLI bundle 308kb.

🤖 Generated with Claude Code

@jack-champagne jack-champagne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI's correct and the gate genuinely fails red — I checked the bash -e errexit path, a malformed config (even a middle one) reds the step. Two changes before merge, both small (inline). Plus the manifest.toml refs here must move in lockstep with #28's run.toml rename.

Tests + Phase 1: two test-debt items vs #17 — the invalid-fixture corpus is one-class-per-file with no wrong-type case (only an in-test mutation covers it; AC7 wants the class matrix self-contained), and the linked amico-validate bin is never exercised in CI (inline). The bigger Phase-1 item: this gate validates only static committed artifacts. Phase 1's reference-prompt smoke corpus IS the real emit→validate gate, and this CI step is where it plugs in — wire one real amico-run → emit run-dir → validate lane here (or in #31/#18), not just configs, so the producer seam is actually covered before Phase 1 piles E2E on top. That's also the infra decision: the cloud solve-service (aws-infra) writes this same run-dir contract to S3, so the gate the cloud track inherits should have seen real emitted output, not just fixtures.

Don't overbuild past this — one ci.yml edit + a build-order fix, not more machinery.

Comment thread .github/workflows/ci.yml
$V packages/extension/scripts/lab.toml.example --schema lab
$V packages/extension/demo/run/manifest.toml
$V packages/extension/demo/run/result.toml
$V packages/extension/demo/run/FINISHED

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[important] the gate covers lab/manifest/result/FINISHED but not solvespec or catalog-entry — AC5 requires the non-filename schemas via --schema, and the DoD is "all five." Both fixtures are committed; add them:

Suggested change
$V packages/extension/demo/run/FINISHED
$V packages/extension/demo/run/FINISHED
$V packages/schema/test/fixtures/valid/solvespec.toml --schema solvespec
$V packages/schema/test/fixtures/valid/catalog-entry.toml --schema catalog-entry

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — the gate now also validates solvespec + catalog-entry via --schema (the committed valid fixtures), so all five are covered (AC5 / DoD).

Comment thread .github/workflows/ci.yml Outdated
run: |
V="node packages/schema/dist/amico-validate.js"
$V packages/extension/scripts/lab.toml.example --schema lab
$V packages/extension/demo/run/manifest.toml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rename] this manifest.toml becomes run.toml when #28 lands — update here in lockstep or the gate validates a filename that no longer exists.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — renamed to run.toml in lockstep with #28.

Comment thread packages/schema/package.json Outdated
"type": "module",
"main": "./src/index.ts",
"types": "./src/index.ts",
"bin": { "amico-validate": "./dist/amico-validate.js" },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[important] this bin doesn't link on a clean install — dist/amico-validate.js is built by esbuild after pnpm install, so install warns Failed to create bin … ENOENT and the amico-validate command is never created. CI misses it because it runs the bundle path directly. For a slice whose deliverable is "ship the CLI," that's the entry point broken. Fix: a prepare script that builds on install, and have CI exercise the linked bin (pnpm exec amico-validate --help) so a broken entry point reds.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in this push — the bin points at a committed launcher (packages/schema/launcher/amico-validate, mirroring amico-run) that exists at install time, so the bin links cleanly (the dist bundle it execs is built by pnpm build). CI now runs pnpm --filter amicode-v2 exec amico-validate --help (the bin links into the dependents, not the root) so a broken entry point reds.

Comment thread packages/schema/src/index.ts Outdated
* --schema. The amico-validate CLI uses this for file-role resolution. */
export function kindForFilename(filePath: string): SchemaKind | undefined {
const base = filePath.replace(/^.*[\\/]/, "");
if (base === "manifest.toml") return "manifest";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[rename] this manifest.toml (+ the SCHEMAS.manifest key, the schema filename, and the ci.yml gate line) must follow #28's run.toml rename together — miss one and basename resolution silently falls through to "cannot infer."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — kindForFilename now resolves run.toml"run", and the SCHEMAS key / schema filename / ci.yml gate / cli.test KINDS all follow #28s rename. Added a self-contained wrong-type fixture so the class matrix no longer leans on an in-test mutation.

@Rchari1
Rchari1 force-pushed the rchari/0.1b-lab-validation branch from 5def952 to ade5a6d Compare June 28, 2026 20:29
Rchari1 and others added 2 commits June 28, 2026 16:29
A user-facing validator over the shared @amicode/schema validate() core, plus a
CI step that reds the PR on a schema-invalid config. No parallel validation logic.

- src/cli.ts: `amico-validate <file> [--schema <kind>]`. Resolves file→schema by
  basename for the fixed-filename artifacts (manifest.toml/result.toml/lab.toml/
  FINISHED via kindForFilename); --schema override for solvespec/catalog-entry or
  any non-standard name. Exit 0 valid / 64 invalid-or-usage (Q85). Field-precise
  stderr (offending key + JSON-pointer path).
- bin amico-validate → dist/amico-validate.js (esbuild, shebang+chmod); ajv bundles.
- test/fixtures/invalid/: one representative malformed file per kind (the shared
  negative corpus); valid corpus from 0.1a is the shared positive corpus.
- test/cli.test.ts (8): valid→0 + invalid→64 across every kind, file-role
  resolution, FINISHED-by-basename, non-filename-needs-schema, field-precise
  stderr, usage errors → 64, --help → 0.
- ci.yml: fast-tier `amico-validate` gate over the SHIPPED configs
  (lab.toml.example + demo/run manifest/result/FINISHED) after build.

schema 32 / amico-run 47 / extension 63 green; CLI bundle 308kb.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…all five, wrong-type fixture, rename lockstep

Address Jack's #30 review (rebased on #28's run.toml rename):
- bin now points at a COMMITTED launcher (packages/schema/launcher/amico-validate,
  mirrors amico-run) instead of the un-built dist/amico-validate.js — so the
  `amico-validate` bin links on a clean install (was "Failed to create bin … ENOENT";
  dist is built by `pnpm build`, absent at install time). CI now exercises the LINKED
  bin (`pnpm --filter amicode-v2 exec amico-validate --help`) so a broken entry reds.
- CI gate validates ALL FIVE: added solvespec + catalog-entry via --schema (AC5 / DoD).
- Self-contained wrong-type fixture (invalid/result-wrongtype.toml) + a cli.test case —
  the class matrix no longer relies on an in-test mutation (AC7).
- Rename lockstep: index.ts kindForFilename + cli.ts usage + ci.yml gate + cli.test
  KINDS + invalid/run.toml all follow #28's manifest.toml→run.toml / "manifest"→"run".

schema 34 / amico-run 47 / extension 64 (+1 skip) green; linked bin + gate verified locally.
(The bigger real-emit→validate lane Jack flagged lands in #31, the round-trip slice.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Rchari1
Rchari1 force-pushed the rchari/0.1c-amico-validate-cli branch from f8daf3f to c8f9863 Compare June 28, 2026 20:35
@Rchari1
Rchari1 requested a review from jack-champagne June 28, 2026 20:35
Rchari1 added a commit that referenced this pull request Jun 28, 2026
…nd-to-end (rebased on #28 rename)

Address Jack's #31 review:
- REAL producer lane (not just fixtures): scripts/producer_roundtrip.sh runs amico-run
  on a minimal non-Piccolo emitter (julia/emit_fixture.jl) so it produces a live run-dir
  (run.toml + FINISHED from the orchestrator, result.toml from the Julia emitter), then
  validates the FRESHLY-EMITTED artifacts with validate.jl. CI runs it (fast tier; the
  real Piccolo solve stays the slow/nightly extension). Scoped to what each side actually
  emits — the orchestrator writes run.toml/FINISHED, the Julia producer writes result.toml.
- AC6 exercised, not asserted: scripts/ac6_drift_check.sh perturbs the SHARED schema,
  REBUILDS TS (it bakes schemas at build time — the gap Jack found), then asserts BOTH the
  TS bin AND validate.jl reject a previously-valid fixture, then reverts. Proven locally;
  wired into the schema-roundtrip CI job. runtests.jl's AC6 testset notes the build-vs-
  runtime asymmetry.
- Rename lockstep: validate.jl + runtests.jl manifest.toml→run.toml, "manifest"→"run",
  run.schema.json; resolved the ci.yml conflict keeping #30's all-five gate + this job.

schema 34 / amico-run 47 / extension 64 (+1 skip) / julia 22 green; producer + AC6
scripts pass locally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@jack-champagne jack-champagne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed c8f9863a. All three resolved:

  • Wrong-type fixture self-containedresult-wrongtype.toml (fidelity="high", a string where a float is required); AC7's class matrix no longer leans on an in-test mutation.
  • Linked bin exercised + gating in CI — the fast job runs pnpm --filter amicode-v2 exec amico-validate --help (a broken bin entry reds) and the committed launcher validates all five schemas under bash -e.
  • manifest.toml → run.toml in lockstep with #28.

Approving. Retargeting to main + merging behind #29.

@jack-champagne
jack-champagne changed the base branch from rchari/0.1b-lab-validation to main June 29, 2026 03:48
@jack-champagne
jack-champagne merged commit 2f9ec39 into main Jun 29, 2026
3 checks passed
@kateebonner kateebonner self-assigned this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants