Skip to content

PLT-464: regression fence — profiles parse + legacy default path#52

Closed
bdchatham wants to merge 2 commits into
mainfrom
brandon2/plt-464-m16-regression-fence-existing-profiles-parse-behave
Closed

PLT-464: regression fence — profiles parse + legacy default path#52
bdchatham wants to merge 2 commits into
mainfrom
brandon2/plt-464-m16-regression-fence-existing-profiles-parse-behave

Conversation

@bdchatham

Copy link
Copy Markdown
Contributor

Implements PLT-464 (M1.6) — a CI fence for the additive-by-construction promise.

What

A config-package test (config/regression_fence_test.go, run by make test/CI — no manual step):

  • Globs every profiles/*.json and asserts each resolves through the binary's real settings path (LoadSettingsResolveSettingsValidate, the viper merge where additive defaults apply) — strictly stronger than the existing raw-unmarshal profiles_test.go.
  • Asserts a no-new-fields config resolves to the legacy default: ArrivalModelClosedLoop, distributions nil — the resolution that gates main's openLoop branch. Asserted against DefaultSettings()/the constant, not literals.

All 8 shipped profiles parse + resolve; a falsification probe confirmed the legacy assertion has teeth (an open_loop config survives as ArrivalModelOpenLoop).

Verify

make verify green (lint-pin + lint 0 issues + test + build + --help + bindings in sync).

🤖 Generated with Claude Code

… (PLT-464)

CI fence for the additive-by-construction promise: glob every profiles/*.json
and assert it resolves through the binary's real settings path
(LoadSettings/ResolveSettings/Validate), and that a no-new-fields config
resolves to the legacy default (ArrivalModelClosedLoop, distributions nil) —
the resolution stage that gates main's open-loop branch. Asserts against the
DefaultSettings()/ArrivalModelClosedLoop constants, not literals.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cursor

cursor Bot commented Jun 15, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only changes with no production config or runtime behavior modified; risk is limited to CI failing if additive defaults or profiles drift.

Overview
Adds PLT-464 CI regression tests in config/regression_fence_test.go so the additive-by-construction config contract stays true: shipped profiles still parse and resolve like the binary, and configs without new workload fields still land on the legacy closed-loop path.

Every profiles/*.json is discovered via glob and run through unmarshal → LoadSettingsResolveSettingsValidate, which is stricter than raw JSON unmarshal alone. Profiles that omit additive knobs must resolve to ArrivalModelClosedLoop with nil key/size distributions; profiles that opt into arrivalModel, maxInFlight, or distributions are skipped for that legacy assertion. A minimal synthetic config pins the same defaults independent of which profiles ship.

registerSettingsFlags is extracted as the shared cobra flag set for viper tests; TestArgumentPrecedence in settings_test.go now uses it instead of duplicating flag declarations.

Reviewed by Cursor Bugbot for commit 95fa12a. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3374877. Configure here.


if probe.Settings.ArrivalModel != "" || probe.Settings.MaxInFlight != nil {
return true
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Legacy fence skips explicit defaults

Medium Severity

usesNewFields treats any present arrivalModel string or maxInFlight JSON key as opting in, including values that only restate legacy defaults (closed_loop, default max-in-flight). Those profiles skip TestProfilesNoNewFieldsResolveToLegacyPath, so the CI fence no longer checks arrival model or nil distributions for them and can hit checked == 0 if every profile documents defaults explicitly.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3374877. Configure here.

…m review)

settings_test.go's inline 18-flag block now calls registerSettingsFlags — a
single source of truth, so the flag set can't drift between the two tests
(the very drift this fence exists to catch).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@bdchatham bdchatham closed this Jun 15, 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.

1 participant