Skip to content

Harden model-spec dependency option cross-checks - #161

Merged
0xShug0 merged 1 commit into
0xShug0:mainfrom
lapy:feature/harden-dependency-schema
Aug 7, 2026
Merged

Harden model-spec dependency option cross-checks#161
0xShug0 merged 1 commit into
0xShug0:mainfrom
lapy:feature/harden-dependency-schema

Conversation

@lapy

@lapy lapy commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Require dependencies[].option to exist under options.<scope>
  • Require required_when[].option_key to refer to a declared option in that condition scope (public key form)
  • Document OR semantics for multiple required_when rows
  • Align the toy Qwen3 ASR demo options with the dependency rows
  • Rebased onto latest main

Follow-up from the discussion on #89. Keeps top-level dependencies separate from options; this only hardens the schema gate.

Test plan

  • model_spec_system_test passes locally (covers undeclared dependencies[].option / required_when.option_key)
  • Spot-check: all 42 in-tree model_specs/*.json satisfy the new dependency/option cross-checks
  • Existing Linux/macOS/Windows/Nix CI builds (no workflow changes in this PR — Linux CI does not currently run unit tests)

@0xShug0

0xShug0 commented Aug 5, 2026

Copy link
Copy Markdown
Owner

@lapy Is this PR ready for review? The test plan lists CI unit tests for the model-spec schema, but that item still appears incomplete.

@lapy
lapy force-pushed the feature/harden-dependency-schema branch from 081c9e5 to 6fe5123 Compare August 6, 2026 16:59
@lapy

lapy commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Yes — ready for review now.

Updates since your comment:

  • Rebased onto latest main (resolved the docs conflict)
  • Wired model_spec_system_test into Linux CI (ENGINE_BUILD_TESTS=ON, CPU matrix)
  • Confirmed the unit test passes locally, and all 42 in-tree model_specs/*.json satisfy the new dependency/option cross-checks

Test-plan checkboxes are updated.

@lapy

lapy commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

CI came back cancelled (no runner / empty steps) after I added a Linux workflow change to run model_spec_system_test. Re-run also refused with “workflow file may be broken” — looks like the fork PR couldn’t execute the modified workflow.

I’ve reverted the .github/workflows change and pushed again so CI uses the existing build workflows only. Schema unit coverage remains in tests/unittests/test_model_spec_system.cpp and passes locally; happy to land a separate follow-up for CI unit tests from a branch with write access if you want that wired in.

Require dependencies[].option and required_when.option_key to reference
declared options, document OR semantics, and keep the toy demo in sync.
@lapy
lapy force-pushed the feature/harden-dependency-schema branch from 0a73c0e to b0ee4b6 Compare August 6, 2026 17:35
@0xShug0
0xShug0 merged commit 98e9c90 into 0xShug0:main Aug 7, 2026
5 of 8 checks passed
@0xShug0

0xShug0 commented Aug 7, 2026

Copy link
Copy Markdown
Owner

@lapy Merged. Thanks!

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.

2 participants