Fix preflight model-family identity across routed models - #4
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (7)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughThe change adds deployment-local model identity declarations and configurable family-diversity policies. Preflight now validates this configuration, resolves exact aliases, records identity sources and policy metadata, and applies the configured diversity threshold. Tests and documentation cover validation, reporting, and read-only behavior. ChangesConfigurable model identity preflight
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The configurable identity resolution and diversity-policy changes are ready to merge. Sequence Diagram(s)sequenceDiagram
participant PreflightRunner
participant ModelIdentityConfig
participant ModelIdentity
participant PreflightReport
PreflightRunner->>ModelIdentityConfig: Parse SCOUT_MODEL_IDENTITY_CONFIG
ModelIdentityConfig-->>PreflightRunner: Validated identities and diversity policy
PreflightRunner->>ModelIdentity: Resolve phase model identifiers
ModelIdentity-->>PreflightRunner: Built-in or declared identities
PreflightRunner->>ModelIdentity: Check family diversity with configured policy
ModelIdentity-->>PreflightRunner: Diversity result
PreflightRunner->>PreflightReport: Record identities, policy, families, and errors
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 5 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🟢 Approval recommended
Changes are scoped, well-tested, and improve correctness of preflight gating with only a minor wording nit in an error message.
Pull request overview
This PR fixes Scout’s preflight model-family gating by separating model identity (developer + family) from routing/transport (e.g., OpenRouter), preventing openrouter/ from being miscounted as a “family” and improving preflight diagnostics.
Changes:
- Introduces a strict model-identity resolver (
resolve_model_identity) that recognizes supported families across direct andopenrouter/<vendor>/<slug>identifiers and fails closed on unknown/opaque aliases. - Updates
run_preflightto computemodel_familiesfrom resolved identities and to emitmodel_identitiesdiagnostics, while preserving read-only DB behavior. - Adds focused tests covering identity resolution, “diversity counts families (not routes/versions)”, and preflight read-only + gating scenarios; documents the recognition boundaries.
File summaries
| File | Description |
|---|---|
| tests/test_model_identity.py | New unit tests for identity resolution and family-diversity behavior. |
| tests/test_load_projects.py | Extends preflight tests to assert identity gating and read-only DB behavior. |
| src/scout/scanning/runner.py | Replaces prefix-splitting family inference with identity-based resolution + diagnostics in preflight. |
| src/scout/model_identity.py | New identity resolver + diversity check used by preflight. |
| docs/configuration.md | Documents preflight diversity semantics and supported identity recognition. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return Err(ModelIdentityError( | ||
| "resolve_model_identity", model, | ||
| "unsupported route, malformed identifier, or opaque alias; " | ||
| "use a recognized direct or openrouter/<developer>/<model> identifier", | ||
| )) |
Summary
SCOUT_MODEL_IDENTITY_CONFIGJSON setting to declare a pipeline-wide minimum of 1–3 families..envconfiguration. Infer transport from the original Jig model identifier; declarations cannot reroute models or contradict known built-in identities.model_families, add built-in versus declared provenance tomodel_identities, and print the effectivemodel_diversity_policy.Scope
Preflight currently treats the
openrouter/transport prefix as a model family,incorrectly collapsing different underlying families into one. This change
separates model identity from routing and diversity policy. It does not select
active models, run paid experiments, or change task authority.
Kimi and Qwen support here means identity recognition, not selection as active models or authorization of experiment candidates. Family recognition does not validate endpoint availability, pricing, credentials, or behavioral qualification.
Validation
uv run pytest -q: 2,069 passed, 11 skipped; existing audioop deprecation warning.uv run ruff check .: passed.uv run mypy .: passed (90 source files).uv build: source distribution and wheel passed.git diff --check: passed.No web code or dependencies changed; web tests were not rerun locally.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation