Skip to content

fix(compat): merge partial model spec overrides - #31

Merged
tuxevil merged 2 commits into
tuxevil:mainfrom
javargasm:feature/fix-partial-model-specs
Sep 4, 2026
Merged

fix(compat): merge partial model spec overrides#31
tuxevil merged 2 commits into
tuxevil:mainfrom
javargasm:feature/fix-partial-model-specs

Conversation

@javargasm

@javargasm javargasm commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to merged #30 for regressions found by the post-merge blind audit and maintainer review:

  • merge partial exact and substring modelSpecs overrides over the effective dynamic, static, or family defaults
  • preserve operator precedence for fields that are provided while inheriting omitted output, thinking, minimum-budget, and context metadata
  • keep required numeric model fields finite and align the TypeScript config contract with the runtime validator's partial-override behavior
  • preserve partial modelSpecs through shared configuration normalization and persisted save/load paths
  • make Antigravity context-window resolution honor the same exact and substring operator overrides used by effective model specs
  • publish inherited dynamic context metadata without changing bundled catalog context-window behavior

Regressions

A partial substring override such as { "gemini-3.8": { "maxOutputTokens": 1000, "isThinking": true } } previously replaced the complete bundled spec. The omitted thinkingBudget reached thinking normalization as undefined, became NaN, and serialized to thinkingBudget: null.

Shared configuration normalization also rebuilt the configuration without modelSpecs, so validated API and persisted configurations silently lost those overrides. Separately, /v1/models reflected a valid contextWindow override while getAntigravityContextWindow() continued returning the bundled value.

Strict TDD evidence

  • RED: the original partial exact/substring override regressions failed before the effective-spec merge fix
  • RED: three maintainer regressions failed with normalized modelSpecs missing and exact/substring context helpers returning 1,000,000 instead of 222,222/333,333
  • GREEN: all focused regressions pass, including persisted partial specs, unknown top-level field filtering, JSON serialization without null thinking budgets, valid zero budgets, and exact/substring context overrides

Verification

  • focused compatibility/catalog/validator suite: 104/104 passed
  • focused configuration/context-window files: 41/41 passed
  • npm run check: 730/730 tests passed
  • source and test typechecks passed
  • lint passed with 15 pre-existing warnings and no errors
  • git diff --check

@tuxevil

tuxevil commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Thanks for the follow-up work. I found one blocking issue that needs to be addressed before merge:

[P1] Preserve modelSpecs through configuration normalization

applyConfigDefaults() rebuilds the configuration object but does not copy config.modelSpecs. Both getCachedConfig() and setCachedConfig() call this function, so a modelSpecs section loaded from accounts.json or submitted through the config API is silently dropped before index.ts calls setModelSpecsOverride(). As a result, the partial-override fix works in direct unit tests but is not usable through the normal persisted configuration path. Please preserve modelSpecs in applyConfigDefaults() and add a regression test covering the validated/normalized configuration path (ideally load/save as well).

I also noticed that the newly accepted contextWindow override is reflected in /v1/models, but getAntigravityContextWindow() still returns the static/dynamic value and ignores the operator override. Please either wire that helper into the same effective-spec resolution or clarify/document that it is intentionally not overrideable.

The current tests and CI checks are green, but I recommend addressing the P1 before merging.

@tuxevil
tuxevil merged commit 8b39db6 into tuxevil:main Sep 4, 2026
2 checks passed
@tuxevil

tuxevil commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Thank you for the follow-up work and for addressing the review findings. The configuration persistence and context-window handling are now covered, and the PR has been merged. Your contribution is appreciated!

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