Skip to content

Add checks file support#178

Merged
andrewh merged 1 commit into
mainfrom
codex/issue-72-checks-file
Jun 13, 2026
Merged

Add checks file support#178
andrewh merged 1 commit into
mainfrom
codex/issue-72-checks-file

Conversation

@andrewh

@andrewh andrewh commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add a separate YAML checks file format for motel check --checks
  • Let checks files define static thresholds and sampled percentile thresholds
  • Keep existing CLI limit flags working, with explicit flags overriding checks-file values
  • Document the checks file shape in the CLI reference

Validation

  • go test ./pkg/synth
  • go test ./cmd/motel
  • make test
  • make lint
  • make build

Fixes #72

@andrewh andrewh changed the title [codex] Add checks file support Add checks file support Jun 13, 2026
@andrewh
andrewh marked this pull request as ready for review June 13, 2026 11:36
@andrewh

andrewh commented Jun 13, 2026

Copy link
Copy Markdown
Owner Author

Adversarial review findings:

  1. P2 pkg/synth/check.go: CheckOptions.Assertions exposes the full CheckThresholds type, including MaxDepth, MaxFanOut, and MaxSpans, but Check() only uses Assertions for percentile checks. The CLI copies static thresholds into legacy fields before calling Check, so the command works, but package callers who load a checks file and pass CheckOptions{Assertions: assertions.Checks} will not get the same behavior. Worse, they will still hit zero-valued MaxDepth/MaxFanOut/MaxSpans unless they also know to copy those fields manually. Either make Check() apply static assertion fields too, or do not expose static thresholds through CheckOptions.Assertions.

  2. P3 pkg/synth/check.go: percentile assertion results drop the selected scenario's Distribution. The CLI therefore prints FAIL p99-depth: ... but not the p50/p95/p99/max line for the scenario that actually caused that percentile assertion. This is especially confusing when the static max-depth result and the percentile assertion choose different scenario sets. Set Distribution: &bestDist on these results so the existing output path prints the supporting distribution.

  3. P3 docs/man/man1/motel.1: the man page is now stale for motel check; it does not include --checks. It was already missing some newer check flags, but this PR adds another user-facing flag and updates only docs/reference/synth.md. If the man page is shipped, this should be updated or deliberately marked/generated differently.

Open question: should --checks accept URLs officially? LoadCheckAssertions uses the shared source reader, so URLs work, but the flag help says "file". Either document URL support or restrict it.

Review scope: reviewed the PR diff against main, checked the published PR body, inspected surrounding check/scenario code, searched for stale CLI docs, and hand-ran a failing percentile check case.

Introduce a separate YAML checks file format for project-specific structural
thresholds. The new `motel check --checks <file-or-url>` flag loads static
limits and sampled percentile limits without requiring assertions to live in
the topology file itself, which keeps imported topologies reusable across
regeneration.

The checks file supports:

- `max_depth`, `max_fan_out`, and `max_spans` for static structural limits
- `p50_*`, `p95_*`, and `p99_*` thresholds for `depth`, `fan_out`, and `spans`
- local files and HTTP/HTTPS URLs, matching the topology source loading model

Move threshold application into `synth.Check` so package callers get the same
behavior as the CLI when passing `CheckOptions{Assertions: ...}`. CLI flags
remain explicit overrides for matching values from the checks source.

Add percentile assertion results as first-class `CheckResult` entries and keep
their supporting distributions attached so command output can explain which
sampled values caused a percentile assertion to fail.

Document the new flag in both the Markdown reference and man page, including
URL support. Add tests for checks-file parsing and validation, URL loading,
CLI pass/fail behavior, CLI override precedence, package-level static
assertion limits, and scenario-aware percentile assertion reporting.

Fixes #72
@andrewh
andrewh force-pushed the codex/issue-72-checks-file branch from 9ea6da2 to 030de49 Compare June 13, 2026 12:02
@andrewh
andrewh merged commit ec64add into main Jun 13, 2026
2 checks passed
@andrewh
andrewh deleted the codex/issue-72-checks-file branch June 13, 2026 12:07
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.

User-defined invariants for motel check

1 participant