Skip to content

feat: add Zero model registry#9

Merged
gnanam1990 merged 2 commits into
mainfrom
feat/zero-model-registry
Jun 2, 2026
Merged

feat: add Zero model registry#9
gnanam1990 merged 2 commits into
mainfrom
feat/zero-model-registry

Conversation

@gnanam1990

Copy link
Copy Markdown
Collaborator

Summary

Adds the first Gnanam-owned M1 runtime module from the balanced WorkSplit PRD: a Zero-branded model registry that downstream /model, /effort, context-budget, provider-runtime, and cost-tracking work can consume.

This PR intentionally keeps the scope to the model registry contract only. It does not introduce provider resolution/factory behavior, Anthropic/Gemini streaming adapters, config changes, or TUI wiring, so it can land before the next Gnanam provider-runtime slices.

What Changed

  • Added src/zero-model-registry/ as a Zero-native backend module.
  • Added typed model metadata for OpenAI, Anthropic, and Google providers.
  • Added 10+ active/preview models with:
    • provider ownership
    • display name and API model id
    • aliases for CLI/config lookup
    • context window and max output limits
    • capabilities such as chat, streaming, tool calling, vision, JSON mode, reasoning, prompt cache, and long context
    • reasoning effort metadata where supported
    • pricing metadata and source verification date
  • Added deprecated-model handling so old config/history can be displayed without polluting normal model selection.
  • Added lookup/filter APIs:
    • listZeroModels
    • resolveZeroModelId
    • getZeroModel
    • requireZeroModel
    • isKnownZeroModel
    • listZeroModelsByProvider
    • listZeroModelsByCapability
    • zeroModelSupportsCapability
    • getZeroReasoningEfforts
    • assertZeroModelProvider
  • Added cost helpers:
    • calculateZeroModelCost
    • formatZeroModelCost
  • Added tier-aware pricing support for long-context models such as GPT-5.5/GPT-5.4 and Gemini Pro.
  • Added Bun test coverage for registry coverage, metadata completeness, aliases, filters, reasoning effort metadata, deprecated-model inclusion, cost math, tiered pricing, and cost formatting.

PRD Alignment

This implements the first Gnanam M1 dependency from Zero-WorkSplit-PRD-v2-balanced:

  • Model registry with 10+ models, cost, context limits, capabilities
  • Supports future Vasanth-owned /model and /effort UI work.
  • Supports future Gnanam-owned provider factory, usage/cost tracking, context budgeting, and config validation.
  • Keeps module naming Zero-specific and avoids generic infrastructure names.

Pricing / Model Sources

Pricing and model metadata were checked against official provider docs on 2026-06-02:

Validation

Local checks run successfully:

  • npx --yes bun test ./tests --timeout 15000
    • 59 pass
    • 0 fail
  • npx --yes tsc --noEmit
    • pass
  • npx --yes bun run build
    • pass
  • ./zero --help
    • pass
  • git diff --check
    • pass

Review Notes

  • The current open provider-catalog PR (Add TOML-backed provider catalog #5) appears adjacent, but this PR does not touch its provider catalog/factory files.
  • This PR keeps the registry as a pure typed module so later provider-resolution work can choose how to consume it without requiring TUI or config changes in this slice.

Add a Zero-branded model registry with provider metadata, aliases, capabilities, context limits, pricing source metadata, and cost helpers.

Cover registry lookup, filters, reasoning effort metadata, deprecated-model handling, tiered pricing, and cost formatting with Bun tests.
@gnanam1990

Copy link
Copy Markdown
Collaborator Author

@Vasanthdev2004 @anandh8x please review this PR when you get a chance. This is the first Gnanam M1 slice: the Zero model registry only, with provider metadata, cost helpers, and validation tests.

@Vasanthdev2004 Vasanthdev2004 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve — clean model registry foundation. Validated tsc clean and 59/59 tests passing; build failure reproduces on origin/main and is not introduced here.

@Vasanthdev2004

Copy link
Copy Markdown
Collaborator

Blockers

None found.

Non-Blocking

  • bun run build still fails on the existing Ink react-devtools-core resolution issue, but I verified the same failure on origin/main, so this is not introduced by this PR.
  • bun run typecheck script is not present on the current base; I validated with bun run tsc --noEmit instead.

Looks Good

  • Clean model-registry slice: metadata, aliases, provider/capability filters, reasoning efforts, pricing tiers, and cost helpers are separated well.
  • Good test coverage for registry lookup, alias resolution, provider/capability filters, tiered pricing, cached input pricing, and cost formatting.
  • Validation in isolated worktree: bun run tsc --noEmit clean, bun test ./tests --timeout 15000 = 59/59 passing.
  • Scope is reasonable: 5 files, +922/-0, no unrelated runtime rewrites.

Verdict: Approve — clean M1 model registry foundation; build blocker is pre-existing on main.

@anandh8x anandh8x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code Review: PR #9feat: add Zero model registry

Author: @gnanam1990feat/zero-model-registrymain
Scope: 5 files, +922/-0. New module src/zero-model-registry/ (types, registry, cost, index) + one test file.
Verdict: Solid foundation with strong typing and good test discipline, but the registry bakes in a large number of speculative / non-existent model IDs and prices, and has a few small correctness/consistency issues that should be fixed before merge. Blocking on the data-side concerns and the silent cache-rate fallback.


Blocking

1. The registry ships speculative model IDs as ground truth

A surprising amount of the catalog appears fabricated. The following IDs have no known public release as of sourceLastVerified: 2026-06-02 (and the listed source URLs do not contain them):

  • OpenAI: gpt-5.5, gpt-5.4, gpt-5.4-mini, gpt-5.4-nano (registry.ts:48-175)
  • Anthropic: claude-opus-4.8, claude-sonnet-4.6, claude-haiku-4.5 (registry.ts:236-297)
  • Google: gemini-3.1-pro-preview, gemini-3-flash-preview, gemini-3.1-flash-lite (registry.ts:320-389)

ZERO_DEFAULT_MODEL_ID = 'gpt-5.4' (registry.ts:44) is also a speculative model, and the test at tests/zero-model-registry.test.ts:66 hard-codes it, so the spec is now entangled with a phantom default.

The real, verifiable models are good: gpt-4.1, gpt-4o, gpt-4-turbo, claude-sonnet-4, gemini-2.5-pro, gemini-2.5-flash, gemini-2.5-flash-lite all have prices that match published docs. But the moment downstream code (provider factory, /model, cost tracking) calls into this registry for a gpt-5.4 request, the system will either fail at the provider layer or, worse, silently send to whatever string the registry hands out.

Ask: Either drop the speculative entries (and the default), or split the file into registry.active.ts (only models with public API access today) and registry.preview.ts (clearly marked, gated behind an experimental flag) so downstream code can refuse to dispatch on them.

2. Misleading aliases for non-existent models

gpt-5.4-mini is aliased to gpt-5-mini and gpt-5.4-nano to gpt-5-nano (registry.ts:140, 161). If a future gpt-5-mini is released, anything that looks up gpt-5-mini will silently keep resolving to gpt-5.4-mini. Same concern for gpt-latest on gpt-5.5 (registry.ts:53).

Ask: Remove *-latest, gpt-5-mini, gpt-5-nano aliases. Document that aliases are stable and never auto-redirect.

3. getCachedInputRate silently charges uncached price for unsupported caching

cost.ts:96-98:

return tier?.cachedInputPerMillion ?? pricing.cachedInputPerMillion ?? inputRate;

For models with no cachedInputPerMillion (e.g., gpt-4-turbo, gemini-3.1-flash-lite, gemini-3.1-pro-preview), any cachedInputTokens in the usage payload is billed at the full input rate. A user who passes cached tokens (a common pattern from streaming usage events) for a model that does not support caching gets charged twice — uncached plus cached, where cached equals uncached.

Ask: Either (a) when no cachedInputPerMillion is set, treat cachedInputTokens as 0 and do not subtract from inputTokens; or (b) throw a clearer error like "Model X does not support prompt caching". Option (a) is the safer default.


Important

4. Tier selection relies on undocumented ordering

cost.ts:73-75:

return pricing.tiers.find(
  (tier) => tier.upToInputTokens === undefined || inputTokens <= tier.upToInputTokens
);

Correct only because every tiers[] in registry.ts happens to be ordered [bounded, unbounded]. A reordering or addition of a mid-tier silently mis-prices prompts.

Ask: Either sort/validate at module load (assert exactly one unbounded tier exists and it is last) or rewrite as an explicit loop with a comment.

5. Inconsistent ID/apiModel separator

Mix of . and - for the same version digit:

  • claude-haiku-4.5 (dot in id) vs claude-haiku-4-5-20251001 (dashes in apiModel) — registry.ts:278-280
  • gpt-5.4 (dot in id) vs gpt-5.4-mini (dots throughout) — registry.ts:81-83

Compare to claude-opus-4.8 / claude-sonnet-4.6 / claude-sonnet-4 which are all dashes. Pick one convention and document it on the type.

6. reasoningEfforts constant is named claudeReasoningEfforts but used for Gemini

registry.ts:38-42 defines claudeReasoningEfforts and it is then reused at registry.ts:347, 367, 388, 419, 440, 461 for gemini-3.1-pro-preview, gemini-3-flash-preview, gemini-2.5-pro, gemini-2.5-flash, gemini-2.5-flash-lite, etc. The gemini-3.1-flash-lite entry at registry.ts:388 even defines its own inline array ['minimal','low','medium','high'] for no documented reason.

Ask: Rename to standardReasoningEfforts (or split into per-provider constants) and use it uniformly. The inline Gemini-3.1-flash-lite array should either be removed or have a comment explaining the deviation.

7. Likely wrong numeric metadata on speculative models

Even setting aside the existence question, several numbers look implausible:

  • gpt-5.5 / gpt-5.4 context.contextWindow: 1_050_000 (registry.ts:54, 87) — 1.05M is a strange number; the rest of the registry uses round values (1_048_576, 1_000_000, 400_000).
  • gpt-5.5 / gpt-5.4 maxOutputTokens: 128_000 (registry.ts:54, 87) — well above any current OpenAI max-output spec.
  • gpt-4.1 maxOutputTokens: 32_768 (registry.ts:183) — published spec is 16,384.
  • claude-haiku-4.5 maxOutputTokens: 64_000 (registry.ts:284) — published Haiku-class max output is 8,192.

If these are intentional projections, the notes field on ZeroModelPricing should mention that.

8. gpt-4-turbo is reachable but unselectable — and untested

listZeroModels() (default) excludes deprecated, but resolveZeroModelId('gpt-4-turbo') still resolves. That is the right design, but there is no test for either the default-exclude or the explicit includeDeprecated: true path on a real consumer (the test at line 22-24 only checks the includeDeprecated: true array contains the ID; it does not assert that the default-exclude path hides it).


Suggestions

9. Missing JSDoc on public API

The functions exported from index.ts are the public surface for downstream /model, /effort, provider factory, and cost tracking. They are well-named but undocumented. At minimum, document the non-obvious ones: calculateZeroModelCost (unit conventions, fallback behavior), selectZeroPricingTier (ordering invariant), assertZeroModelProvider (when to use vs. requireZeroModel).

10. Test coverage gaps

Easy wins:

  • requireZeroModel error path (unknown alias throws).
  • assertZeroModelProvider error path (provider mismatch throws).
  • formatZeroModelCost invalid input (negative, NaN).
  • calculateZeroModelCost with a ZeroModelDefinition object (not a string) — cost.ts:17 supports it, but no test exercises it.
  • calculateZeroModelCost for a model with no cachedInputPerMillion (currently uncovered; once #3 is fixed, this is the test that catches the regression).
  • listZeroModels() default (no args) must not return gpt-4-turbo.
  • A test that demonstrates the input/cached split: with inputTokens: 1_000_000, cachedInputTokens: 1_000_000, the cached input should consume the entire input budget and inputCost should be 0.

11. notes field on ZeroModelPricingTier is write-only

ZeroModelPricingTier.note is defined and populated but never read. Either surface it (e.g., return it on ZeroModelCostBreakdown alongside pricingTier) or drop it until it is needed.

12. nonNegativeInteger rejects valid floats

cost.ts:114-118 throws if value is not an integer. Provider usage events frequently emit float token counts (e.g., 1234.0 for streaming aggregations). Consider rounding instead of throwing, or accepting Math.floor semantics.

13. getZeroModel performs redundant normalization

registry.ts:504:

return modelsById.get(normalizeModelKey(modelId));

resolveZeroModelId already returns a canonical id that is a key in modelsById (since it is set in the bootstrap loop). The normalizeModelKey here is defensive but adds noise. Either drop it or add a comment that the canonical id may still have whitespace/case in some future path.

14. listZeroModelsByProvider / listZeroModelsByCapability re-filter per call

Both go through listZeroModels() and create intermediate arrays. For 20 models this is fine, but if the registry grows, precompute provider/capability indexes at module load next to modelsById / aliasesByName. Minor.


Questions for the author

  1. Are the speculative OpenAI/Anthropic/Google entries real private betas you have access to, or projections? If projections, what is the consumer contract — should /model refuse to dispatch on a preview/active model whose apiModel string is not confirmed by a provider SDK?
  2. Why is the default gpt-5.4 and not one of the verified models (gpt-4.1 is the closest "long-context, non-reasoning" stable; claude-sonnet-4 is the closest "balanced, stable")?
  3. Is there a process planned to update sourceLastVerified and prices on a schedule, or is this a snapshot?
  4. The PR description references Zero-WorkSplit-PRD-v2-balanced. Can you link the M1 model-registry requirement? Reviewers outside the team cannot validate the slice fit.

Validation

The PR body's local checks (bun test 59/0, tsc --noEmit, bun run build) are the right set and the new tests pass. The math for the three cost tests was re-verified by hand and holds up: the tier-2 selection for gpt-5.4 at 1M input correctly applies input 5, cached 0.5, output 22.5 → 4.5 + 0.05 + 11.25 = 15.8. formatZeroModelCost outputs are correct for the two cases tested.


Recommendation

Request changes. The data-accuracy and silent-cache-fallback issues are blockers. The naming/constant-cleanup items and test gaps are follow-ups that can land in the same PR or as a quick follow-up. Once the speculative entries are either removed or quarantined behind a preview API surface, and the cache-rate fallback is fixed, this is a clean, mergeable foundation for the M1 dependency.

Comment thread src/zero-model-registry/registry.ts Outdated
'high',
] as const satisfies readonly ZeroReasoningEffort[];

export const ZERO_DEFAULT_MODEL_ID = 'gpt-5.4';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking — speculative default model.

ZERO_DEFAULT_MODEL_ID = 'gpt-5.4' points to a model that is not publicly released as of sourceLastVerified: 2026-06-02. The test at tests/zero-model-registry.test.ts:66 hard-codes this default, so the rest of the system is now anchored to a phantom model.

If gpt-5.4 is a private beta you have access to, gate it behind a feature flag and add a note. Otherwise default to a verified stable model (e.g., gpt-4.1 or claude-sonnet-4).

Comment thread src/zero-model-registry/registry.ts Outdated

export const ZERO_MODEL_REGISTRY = [
{
id: 'gpt-5.5',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking — speculative OpenAI models as ground truth.

gpt-5.5, gpt-5.4, gpt-5.4-mini, and gpt-5.4-nano are not in PRICING_SOURCE.openai as of the verification date. The provider factory and /model command will look these up by id and silently dispatch to whatever string this registry hands out.

Ask: Either remove these entries, or split the file into registry.active.ts (only models with public API access today) and registry.preview.ts (clearly marked, gated behind an experimental flag) so downstream code can refuse to dispatch on them. Same applies to the Anthropic and Google speculative entries below.

Comment thread src/zero-model-registry/registry.ts Outdated
apiModel: 'gpt-5.5',
provider: 'openai',
status: 'active',
aliases: ['openai:gpt-5.5', 'gpt-5.5-latest', 'gpt-latest'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking — misleading alias.

gpt-latest on gpt-5.5 is a time-bomb: once a newer gpt-* ships, anything resolving gpt-latest will silently keep pointing at gpt-5.5 forever. Remove *-latest aliases and document that aliases are stable and never auto-redirect.

Comment thread src/zero-model-registry/registry.ts Outdated
apiModel: 'gpt-5.4-mini',
provider: 'openai',
status: 'active',
aliases: ['openai:gpt-5.4-mini', 'gpt-5-mini'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking — misleading alias.

gpt-5-mini aliasing to gpt-5.4-mini is a false promise: if a real gpt-5-mini is ever released, lookups for gpt-5-mini will silently keep resolving to gpt-5.4-mini. Same for gpt-5-nanogpt-5.4-nano on line 161. Drop these aliases.

Comment thread src/zero-model-registry/registry.ts Outdated
description: 'Deprecated OpenAI model retained for config migration and history display.',
},
{
id: 'claude-opus-4.8',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocking — speculative Anthropic models.

claude-opus-4.8, claude-sonnet-4.6, and claude-haiku-4.5 are not publicly released. PRICING_SOURCE.anthropic does not list them. The claude-haiku-4-5-20251001 apiModel also implies a snapshot date that does not exist.

Ask: Remove or quarantine behind a preview flag, same as the OpenAI comment above.

provider: 'openai',
status: 'active',
aliases: ['openai:gpt-4.1'],
context: { contextWindow: 1_047_576, maxOutputTokens: 32_768 },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Important — likely wrong max output.

gpt-4.1 maxOutputTokens: 32_768 — published GPT-4.1 max output is 16,384. Verify against https://platform.openai.com/docs/pricing/ and fix.

provider: 'anthropic',
status: 'active',
aliases: ['anthropic:claude-haiku-4.5', 'haiku-4.5'],
context: { contextWindow: 200_000, maxOutputTokens: 64_000 },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Important — likely wrong max output.

claude-haiku-4.5 maxOutputTokens: 64_000 — published Haiku-class max output is 8,192. If this is a speculative model and the number is also speculative, the notes field on ZeroModelPricing should call that out.

const models = listZeroModels();
expect(models.length).toBeGreaterThanOrEqual(10);
expect(models.some((model) => model.status === 'deprecated')).toBe(false);
expect(listZeroModels({ includeDeprecated: true }).some(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Important — default-exclude path is untested.

This only asserts that gpt-4-turbo exists when includeDeprecated: true. There is no test that listZeroModels() (default, no args) excludes it. Add one — it's a one-liner and protects the public surface.

Comment thread src/zero-model-registry/registry.ts Outdated
export function getZeroModel(modelOrAlias: string): ZeroModelDefinition | undefined {
const modelId = resolveZeroModelId(modelOrAlias);
if (!modelId) return undefined;
return modelsById.get(normalizeModelKey(modelId));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion — redundant normalization.

modelsById.get(normalizeModelKey(modelId))resolveZeroModelId already returns a canonical id that was inserted via normalizeModelKey in the bootstrap loop, so the second normalize is defensive but adds noise. Either drop it or add a one-line comment explaining why the canonical id may still need normalization.

return rate;
}

function nonNegativeInteger(value: number, label: string): number {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion — nonNegativeInteger rejects valid floats.

Number.isInteger rejects 1234.0 even though that's a legitimate streaming token count. Consider Math.floor semantics instead, or document that callers must pre-round. The current test only uses integers, so this is a silent breaking change for any provider that emits float counts.

Remove speculative model entries and unstable latest-style aliases from the Zero model registry.

Use verified public model metadata, default to gpt-4.1, document stable alias semantics, and tighten cached-token cost handling.

Expand tests for deprecated filtering, alias stability, provider mismatch errors, direct model cost inputs, unsupported cache pricing, and invalid cost formatting.
@gnanam1990

Copy link
Copy Markdown
Collaborator Author

@anandh8x requested changes addressed in bfc9bff.\n\nSummary:\n- Removed speculative/non-public registry entries and phantom default model.\n- Changed Zero default model to verified public gpt-4.1.\n- Removed unstable/latest-style aliases and documented stable alias semantics.\n- Kept registry to verified public OpenAI/Anthropic/Gemini models only.\n- Fixed cached-token pricing so models without cache pricing treat cached tokens as ordinary input rather than charging them through a fake cache rate.\n- Reworked tier selection so pricing no longer depends on declared tier order.\n- Renamed shared reasoning effort constant and removed the provider-mismatched naming.\n- Fixed reviewed numeric metadata: gpt-4.1 max output and Haiku max output.\n- Added tests for deprecated filtering, alias stability, required-model errors, provider mismatch, direct model object cost calculation, unsupported cache pricing, full cached-input pricing, invalid cost formatting, and floored token counts.\n\nValidation:\n- npx --yes bun test ./tests --timeout 15000 -> 63 pass, 0 fail\n- npx --yes tsc --noEmit -> pass\n- npx --yes bun run build -> pass\n- ./zero --help -> pass\n- git diff --check -> pass\n\nPlease rereview when you get a chance.

@anandh8x anandh8x left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review: PR #9feat: add Zero model registry (commit bfc9bff9)

Thanks for the thorough response. Diff is -228/+121 on registry.ts, +38/-13 on cost.ts, +4/-0 on types.ts, +60/-17 on tests. Net cleanup of ~91 lines. All three blocking issues are resolved, and the registry is now a defensible ground-truth catalog.

Resolved

Blocking

  • #1 Speculative models removed. Catalog is now only public, verifiable models: gpt-4.1, gpt-4.1-mini, gpt-4.1-nano, gpt-4o, gpt-4o-mini, gpt-4-turbo (deprecated), claude-opus-4.1, claude-opus-4, claude-sonnet-4.5, claude-sonnet-4, claude-haiku-4.5, claude-haiku-3.5, gemini-2.5-pro, gemini-2.5-flash, gemini-2.5-flash-lite. ZERO_DEFAULT_MODEL_ID is now gpt-4.1 (registry.ts:29). Spot-checked every line against published docs — all correct.
  • #2 Misleading aliases removed. gpt-latest, gpt-5-mini, gpt-5-nano, gemini-2.5-pro-latest are gone, and ZeroModelDefinition.aliases now has an explicit contract: "Stable aliases only; aliases must never auto-redirect to a future model." (types.ts:58). The negative test at tests/zero-model-registry.test.ts:57-61 locks that contract in.
  • #3 Cache-rate fallback tightened. cost.ts:34-46 now treats cachedInputTokens as 0 when the model exposes no cachedInputPerMillion, and cost.ts:118-123 returns undefined rather than falling back to inputRate. Test at tests/zero-model-registry.test.ts:132-143 covers the regression.

Important

  • #4 Tier selection rewritten. cost.ts:78-102 now sorts bounded tiers and uses an explicit loop with a throw if no tier covers. The invariant is also documented on ZeroModelPricingTier.upToInputTokens ("omit only on the final fallback tier").
  • #6 claudeReasoningEffortsstandardReasoningEfforts (registry.ts:23-27) and applied uniformly. The inline Gemini-3.1-flash-lite array is gone with the model itself.
  • #7 Numeric metadata corrected. gpt-4.1 max output is now 16,384 (line 39), claude-haiku-4.5 is 8,192 (line 246). The 1,050,000-context oddities are gone with the speculative entries.
  • #8 Deprecated filter coverage. tests/zero-model-registry.test.ts:24-27 now asserts both the default-excludes and includeDeprecated: true-includes paths for gpt-4-turbo.

Suggestions

  • #9 JSDoc added on the non-obvious exports: requireZeroModel (line 398), assertZeroModelProvider (line 434), calculateZeroModelCost (lines 12-19), and on id / apiModel / aliases / upToInputTokens in types.ts.
  • #10 Tests expanded for all the gaps I called out: unknown-model throw, provider mismatch throw, invalid cost throw, ZeroModelDefinition object input, fully-cached input, alias stability, deprecated filtering.
  • #12 nonNegativeInteger accepts floats via Math.floor (cost.ts:139-143); the new haiku-3.5 test exercises 2_000.92_000.
  • #13 Redundant normalizeModelKey in getZeroModel removed (registry.ts:395).

I hand-verified the cost math on the new test cases:

  • gpt-4.1 1M input / 100k cached / 500k output → 1.8 + 0.05 + 4.0 = 5.85 ✓
  • gpt-4.1-mini 1M / 1M cached / 0 output → input 0, cached 0.1, total 0.1 ✓
  • haiku-3.5 2_000.9 prompt / 1_000.1 completion → 0.0016 + 0.004 = 0.0056 ✓
  • gpt-4-turbo 1k input / 1k cached / 1k output → cache dropped, input 0.01, output 0.03 ✓
  • gemini-2.5-pro tier boundary at 200k → tier 1 = 1.25, tier 2 = 2.5 ✓

Minor (non-blocking)

  • #5 ID/apiModel separator. claude-haiku-4.5 / claude-sonnet-4.5 IDs use dots while their apiModels (claude-haiku-4-5-20251001, claude-sonnet-4-5-20250929) use dashes. The new JSDoc on id ("Version dots are kept when the provider markets the model that way") reads as defensive but the displayName in the Anthropic docs uses dots ("Claude Haiku 4.5") and the apiModel uses dashes. The current state is consistent with that split — call it intentional, but worth a one-line follow-up: either rename IDs to claude-haiku-4-5 for symmetry or add a displayName-based rule to the JSDoc.
  • #11 ZeroModelPricingTier.note still write-only. Populated at registry.ts:298, 303 (and notes[] on ZeroModelPricing is populated for every Anthropic model with the cache-rate caveat) but never surfaced on ZeroModelCostBreakdown. Either expose on the breakdown or drop. Nit.
  • #14 listZeroModelsByProvider / listZeroModelsByCapability re-filter per call. Still there, still fine at 14 models. Skip.

Questions (follow-up, not blocking)

  • The Anthropic notes: ['cachedInputPerMillion models Claude cache hit and refresh pricing.'] is repeated on all six Anthropic models. Worth hoisting to the registry level (or to ZeroModelCapability = 'prompt-cache') once you wire the cost tracking to actually surface notes — otherwise it's documentation in disguise.
  • When the next Anthropic model lands, the claude-opus-4.1 id is 4.1 not 4-1. Are we committing to the dot convention for new entries, or is each new entry a fresh argument? Worth pinning in types.ts JSDoc so the next contributor doesn't relitigate it.

Recommendation

Approve. All blocking issues from the previous review are resolved, the important items are fixed or have a documented contract, and the test coverage is now strong enough to catch regressions on the items that were broken. The remaining items are nits and a follow-up tracking issue, not blockers for the M1 dependency.

Nice cleanup.

description: 'Anthropic stable Sonnet model for provider compatibility.',
},
{
id: 'claude-haiku-4.5',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit — ID/apiModel separator. claude-haiku-4.5 (id, dot) vs claude-haiku-4-5-20251001 (apiModel, dashes) — and same for claude-sonnet-4.5 / claude-sonnet-4-5-20250929 at line 196. The new JSDoc on id is a useful contract, but the reason claude-haiku-4.5 uses a dot is that Anthropic's display name uses a dot, not because the API model does. Either rename the IDs to claude-haiku-4-5 / claude-sonnet-4-5 for symmetry with the apiModel, or tighten the JSDoc to call out that the dot is from the display name. Not blocking — calling it out so the next contributor knows which convention to apply.

outputPerMillion: 75,
source: PRICING_SOURCE.anthropic,
sourceLastVerified: SOURCE_LAST_VERIFIED,
notes: ['cachedInputPerMillion models Claude cache hit and refresh pricing.'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit — repeated notes. notes: ['cachedInputPerMillion models Claude cache hit and refresh pricing.'] is duplicated on every Anthropic model (lines 167, 189, 211, 233, 255, 277). When the cost breakdown is wired to surface pricingTier.note or pricing.notes, this duplication will be a maintenance smell. Consider hoisting the cache-pricing caveat to a constant next to PRICING_SOURCE (const ANTHROPIC_CACHE_NOTE = ...) and referencing it on each entry, or attaching the note to the 'prompt-cache' capability itself.

}

const fallbackTier = pricing.tiers.find(
(tier) => tier.upToInputTokens === undefined

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion — throw path is untested. selectZeroPricingTier now throws "No Zero model pricing tier covers ${inputTokens} input tokens" when no tier covers the input (line 98). All current models either have an unbounded tier or no tiers at all, so this branch is unreachable today, but it's a useful safety net. One-line test:

expect(() =>
  calculateZeroModelCost({ ...requireZeroModel('gpt-4.1'), pricing: { ...requireZeroModel('gpt-4.1').pricing, tiers: [{ inputPerMillion: 1, outputPerMillion: 2, upToInputTokens: 100 }] } }, { inputTokens: 101 })
).toThrow('No Zero model pricing tier covers');

Or just add it when the next tier-using model is added.

}

export interface ZeroModelPricingTier {
/** Inclusive input-token ceiling for this tier; omit only on the final fallback tier. */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion — the upToInputTokens invariant is now in two places. The JSDoc here ("omit only on the final fallback tier") and the runtime check in selectZeroPricingTier (cost.ts:97-99) both encode the same invariant. If you add a module-load validator that asserts at most one unbounded tier exists and that the input-rate coverage chain is total, you can drop the JSDoc and let the error speak for itself. Minor — current state is fine.

@gnanam1990
gnanam1990 merged commit abab103 into main Jun 2, 2026

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found a couple of model-metadata issues that should be corrected before this registry becomes the source of truth for downstream routing, budgeting, and cost tracking.

Findings

  • [P2] Add Gemini cached-input pricing to the tiered entries
    src/zero-model-registry/registry.ts:293
    Gemini 2.5 Pro has published context-caching prices for both <=200k and >200k prompt tiers, but the tier objects only include input/output rates. Because calculateZeroModelCost treats cachedInputTokens as zero whenever the selected tier has no cachedInputPerMillion, any Gemini Pro usage event with cached tokens is charged as fully uncached input. The same cached-pricing metadata also needs a source-doc pass for Flash-Lite: this entry uses 0.025, while the current Gemini pricing page lists the text/image/video cache rate as 0.01. Please add the tier-specific cache rates and cover a cached Gemini Pro/Flash-Lite cost case so this does not silently drift again.

  • [P2] Correct GPT-4.1 max output metadata
    src/zero-model-registry/registry.ts:39
    The registry marks gpt-4.1 as maxOutputTokens: 16_384, but the current OpenAI model page for GPT-4.1 lists a 32,768-token max output. Since this registry is intended to drive context budgets and model selection, downstream consumers would unnecessarily cap GPT-4.1 output below the provider limit. Please update the metadata and add an assertion for the published value instead of only checking that the number is positive.

  • [P2] Avoid making every provider/gateway addition a source edit
    src/zero-model-registry/types.ts:1
    The new registry hard-codes the supported provider ids in the ZeroModelProvider union and stores all model/provider metadata in TypeScript source. That means adding a new first-class gateway or provider later is not just a catalog/data change: it requires editing shared source types, registry entries, tests, and any downstream provider/runtime branches that consume this union. This is exactly the kind of maintenance path that will make gateway support drift over time, especially once OpenRouter/OpenGateway/Groq/Ollama-style entries need provider-specific credentials, discovery, auth headers, or transport overrides. Please either keep this module explicitly scoped to first-party model metadata, or move provider/gateway definitions to a data-backed descriptor/catalog surface so adding a gateway does not require coordinated edits across core source each time.

gnanam1990 added a commit that referenced this pull request Jun 27, 2026
Addresses the correctness findings from the PR review.

self-report (#1): drop behavior-describing phrases ("fall back to",
"placeholder value", bare "best guess", "as a fallback", "without proper")
that also match legitimate final answers; keep first-person/uncertainty
admissions only, since these are matched without a context guard.

self-report (#5): scan every occurrence of each inability stem so an early
success-negation ("could not find any examples") no longer masks a later
genuine admission with the same stem ("could not implement it").

continuation cue (#6): require a trailing colon AND an action lead-in on the
final clause; stop flagging recommendations, plain summary colons, and
sign-offs. Still catches the mid-line "...Let me check the config:".

gate order (#8): check the self-report admission BEFORE pending-plan, so an
admitted-impossible task downgrades immediately with the accurate reason
instead of burning continue-nudges.

pending plan (#3): treat a pending/in_progress update_plan item as a
NUDGE-only weak signal -- it no longer forces INCOMPLETE on its own (a
completed run that left stale plan bookkeeping is trusted). Only a
continuation cue or a self-report admission finalizes INCOMPLETE.

max-turns (#4): a run cut off at the MaxTurns ceiling now finalizes
INCOMPLETE under the gate instead of being reported as success.

exec json/cron (#2, #7): for -o json, emit the terminal done with exit 4 on
an incomplete run (final() pre-emits a success done:0 for json that would
otherwise mask it); emit an error event -- not just a warning -- so the cron
failure extractor can recover the reason.

Deferred (noted on the PR): acceptance-only-when-mutated (#9, cost) and
reusing tools.normalizePlanStatus (#10, would widen scope to internal/tools).

Tests: add TestContinuationCueMatching and TestMaxTurnsCutoffIsIncompleteUnderGate,
extend TestSelfReportedIncompletionMatching with the #1/#5 cases, and replace
the in_progress=>incomplete test with TestPendingPlanAloneDoesNotForceIncomplete.
make build / go vet / make lint / go test ./... -race all green.
gnanam1990 added a commit that referenced this pull request Jun 27, 2026
… on no-tool-call / self-reported-incomplete turns) (#325)

* fix(agent): don't end a run as success on a no-tool-call turn mid-task

A turn that produced text but no tool call was always accepted as the final answer, so the loop reported success even when the model stopped mid-task (e.g. ended on "...Let me check the SSH configuration:" with plan steps still pending).

Add an opt-in completion gate (Options.RequireCompletionSignal): when a turn has no tool call and work clearly remains -- pending update_plan items, or the message ends on a continuation cue -- re-prompt the model to continue instead of finalizing. Bounded by maxContinueNudges and still by MaxTurns/the deadline; once the budget is spent the run finalizes as INCOMPLETE (Result.Incomplete) rather than success. Default off, so the interactive path is byte-identical.

Genuine single-turn completions (no pending plan, no cue) still finalize as success. Covered by internal/agent/completion_gate_test.go.

* feat(exec): report stalled headless runs as INCOMPLETE (exit 4)

Enable the agent completion gate for headless exec (RequireCompletionSignal) and map Result.Incomplete to run_end status "incomplete" with a new exit code 4, so a run that stalled mid-task (model stopped without a tool call while work remained, continue budget exhausted) is no longer reported as success. Interactive callers are unaffected.

* fix(agent): downgrade self-reported non-completion; advisory task-grounded acceptance

Reduce -- not eliminate -- false-success on headless runs. Two DETERMINISTIC, unit-tested gates (with the plan gate from the prior commit):

(a) self-report downgrade: if the final message admits the model guessed or could not meet the objective, finalize INCOMPLETE (exit 4), never success. Inability is matched by first-person STEMS generalized over verb/tense ("I cannot/can't/could not/am unable to/do not have/unable to ...") plus guess/fallback/uncertainty phrases, with a guard so success-y negations ("could not find any issues", "cannot reproduce") are not misread. (b7bc0b8's plan gate already forces INCOMPLETE on pending/in_progress update_plan items at termination.)

(b) task-grounded acceptance is ADVISORY, not a guarantee. When --self-correct is on it demands one bounded acceptance pass that re-derives the task's stated criterion and runs a concrete check, discouraging three false-success patterns (well-formed==correct, existing-tests-pass==objective-met, result==baseline-it-was-told-to-beat). But it is a prompt: a model that ignores it and confidently claims "PASS, all requirements met" still slips, because ZERO has no general oracle to verify correctness against a task's hidden criterion. Empirically (TB-2, qwen3-coder:480b) this reliably catches admissions and incomplete plans and REDUCES false-success, but a confident false PASS on a model-ceiling task is a residual, fundamental gap -- not a tuning miss.

Default off (RequireCompletionSignal); interactive callers are byte-identical. Covered by internal/agent/{acceptance_gate_test.go,completion_gate_test.go}.

* feat(exec): surface the INCOMPLETE reason in run_end and logs

When a headless run finalizes as INCOMPLETE, include Result.IncompleteReason in the session error event and a stderr warning so an honestly-incomplete run (e.g. "the final message admits the objective was not met") is debuggable rather than an opaque exit 4.

* fix(agent): harden completion gate per PR review

Addresses the correctness findings from the PR review.

self-report (#1): drop behavior-describing phrases ("fall back to",
"placeholder value", bare "best guess", "as a fallback", "without proper")
that also match legitimate final answers; keep first-person/uncertainty
admissions only, since these are matched without a context guard.

self-report (#5): scan every occurrence of each inability stem so an early
success-negation ("could not find any examples") no longer masks a later
genuine admission with the same stem ("could not implement it").

continuation cue (#6): require a trailing colon AND an action lead-in on the
final clause; stop flagging recommendations, plain summary colons, and
sign-offs. Still catches the mid-line "...Let me check the config:".

gate order (#8): check the self-report admission BEFORE pending-plan, so an
admitted-impossible task downgrades immediately with the accurate reason
instead of burning continue-nudges.

pending plan (#3): treat a pending/in_progress update_plan item as a
NUDGE-only weak signal -- it no longer forces INCOMPLETE on its own (a
completed run that left stale plan bookkeeping is trusted). Only a
continuation cue or a self-report admission finalizes INCOMPLETE.

max-turns (#4): a run cut off at the MaxTurns ceiling now finalizes
INCOMPLETE under the gate instead of being reported as success.

exec json/cron (#2, #7): for -o json, emit the terminal done with exit 4 on
an incomplete run (final() pre-emits a success done:0 for json that would
otherwise mask it); emit an error event -- not just a warning -- so the cron
failure extractor can recover the reason.

Deferred (noted on the PR): acceptance-only-when-mutated (#9, cost) and
reusing tools.normalizePlanStatus (#10, would widen scope to internal/tools).

Tests: add TestContinuationCueMatching and TestMaxTurnsCutoffIsIncompleteUnderGate,
extend TestSelfReportedIncompletionMatching with the #1/#5 cases, and replace
the in_progress=>incomplete test with TestPendingPlanAloneDoesNotForceIncomplete.
make build / go vet / make lint / go test ./... -race all green.
@Vasanthdev2004
Vasanthdev2004 deleted the feat/zero-model-registry branch June 28, 2026 08:27
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.

4 participants