Skip to content

PAYG remap: format, parser, and conformance vectors for plan-billed models - #14

Open
iceteaSA wants to merge 7 commits into
cortexkit:masterfrom
iceteaSA:payg-remap
Open

PAYG remap: format, parser, and conformance vectors for plan-billed models#14
iceteaSA wants to merge 7 commits into
cortexkit:masterfrom
iceteaSA:payg-remap

Conversation

@iceteaSA

@iceteaSA iceteaSA commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

PAYG remap: format, parser, and conformance vectors for plan-billed models

Refs cortexkit/astrocyte#3.

What this is

models.dev publishes cost: {input: 0, output: 0} for plan-billed lanes — 486 models
across 60 providers in the 2026-08-13 snapshot. Those zeros are correct as marginal cost
and useless for routing: a spend report that prices plan usage at $0 cannot answer "what
would this call have cost on this platform without the plan", which is the question that
decides where work goes.

This adds a document format and parser that overlays the catalog with sourced rates for
those ids, plus conformance vectors that define what a correct overlay does.

It does NOT add a classifier. See "What is not here".

What is here

  • PaygRemapDoc and friends — the document types, in a new payg_remap module.
  • An exact provider-qualified key newtype with no fallback to a bare model name.
  • A fallible parser with 11 error variants, all reachable and tested.
  • A conformance runner generic over the join, in payg_conformance.
  • Two vector corpora under tests/golden/, following the pattern in
    cortexkit-store-types and cortexkit-cache-core.

Additive: no existing type, function, or test changes. The only edit to lib.rs is nine
export lines; the only deletion in the diff is the version bump to 0.3.0.

Override costs reuse the existing CostSchedule rather than a parallel type, and rates go
through the existing decimal_str_to_nanos — one money representation, and the private
helper stays private.

What is not here, deliberately

No classifier. Nothing in this crate takes a remap document plus a catalog and returns
an outcome. The classification rules are specified as a matrix and shipped as executable
vectors, but the join itself is not implemented here.

The failure taxonomy is still moving. It grew a third mode after one review round, gained
four matrix cells after another, and had its whole "priced" column restructured after a
third. A classifier in this crate would pin that taxonomy to this crate's semver surface
while it is still changing, and a #[cfg(test)] reference implementation would be worse:
as the only executable join in the tree it becomes the de facto normative one, because that
is what people copy.

So the runner is generic over Fn(&PaygRemapDoc, &CatalogDoc, &PaygModelId) -> PaygOutcome
and this crate provides no value of that type. Whoever writes the classifier gets the whole
suite executable in one call. Where it should live is the open question on astrocyte#3.

No data document. payg-remap.json is not in this PR. The crate header says "types and
parsing only, NO bundled data", and I did not want to be the first exception. Where the
canonical document lives is a placement question that belongs with you.

Two gates, and only one of them runs here

The split is explicit:

gate what it covers executed
parse document syntax, kinds, provenance, rate parsing, the structural Default guard here, in this PR
classification the outcome matrix, the all-zero predicate as applied, refusal directions at the classifier's home, via the shipped runner

The parse gate is proven by mutation: each of the 14 guards was deleted and separately
narrowed, and each mutation reddens a named vector. The classification suite is complete
and cell-referenced but does not execute here, because there is nothing to execute it
against. Seventeen of the 31 mutation rows are shipped and unrun until a classifier exists.

What mutation testing found

The first pass ran every mutation as "delete the guard" and reported 14/14 reddened. An
independent reviewer then ran the narrowing class — leave the guard, make it check less —
and five guards survived:

  • the provenance empty-string filter had never executed at all, because every vector
    omitted the field and the lookup short-circuited before reaching it
  • all_zero narrowed to a single field survived, because no positive test proved it does
    not over-fire
  • the schema gate rejected only newer schemas, so schema: 0 passed
  • the id guard rejected only an empty provider, so provider/ passed
  • chained-target could not distinguish checking the target from checking the source

All five were correct, load-bearing code with no test behind them. They are pinned now.
The vectors also encode the resulting rule: a refusal predicate needs both directions, and
a negative vector that omits a field cannot pin a guard that validates the field's contents.

Vector design

Each classification vector carries a cell reference naming the matrix cell it derives
from. A vector whose expected outcome contradicts its cited cell is then catchable by
reading, without executing anything — the matrix is the oracle. The well-formedness test
enforces that every reference resolves and that all 29 cells are covered exactly once.

29 rather than 20: the matrix prints 5 declarations × 4 source states, but the three
resolves_to "by target" cells each expand over the target's own four states.

The test file's doc comment carries the obligation: any classifier implementation must
execute this suite through run_vectors, and one that does not is nonconforming.

Verification

cargo test -p cortexkit-model-catalog     # 15 unit + 7 + 2 + 29 integration + 1 compile-fail doctest
cargo clippy -p cortexkit-model-catalog -- -D warnings

The compile-fail doctest is the structural guard: PaygRemapDoc derives no Default and
parsing is fallible with no infallible constructor, so unwrap_or_default() does not
compile. That is deliberate — a remap document that silently defaults to empty would
reinstate every false zero it exists to remove.

Open questions for you

  1. Where the classifier lives as the taxonomy grows (astrocyte#3).
  2. Where payg-remap.json lives, given the crate is deliberately data-free.
  3. Whether supersession detection belongs in fusiform. DeclarationSuperseded is a
    catalog-era transition — "this id started being priced" — and fusiform's diff pipeline
    already computes that event. Related: a classification is only reproducible against the
    catalog read it came from, so a consumer should record that read's resolved_at_ms
    rather than keying on catalog_version, which advances on its own clock.

Design notes, including the failure modes this cannot represent, are in the astrocyte#3
thread.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds a PAYG remap format, parser, and conformance vectors so plan‑billed models can be priced counterfactually. Previously plan lanes had $0 marginal cost; now an overlay supplies sourced rates or refusal rules (including explicit rate_time_banded) with an optional effective_from (validated YYYY‑MM‑DD and accepted when null). Behavior does not change until a classifier consumes it.

  • Adds payg_remap and payg_conformance to cortexkit-model-catalog; exports Payg* types, run_vectors, PaygOutcome, ResolvesToEntry, OverridesUnpricedEntry, NotSoldPerTokenEntry, RateTimeBandedEntry, and the normative is_all_zero; bumps crate to 0.3.0 (additive).
  • Parser (schema 1) requires counterfactual: "same_platform_list", exact provider/model ids, and provenance; validates effective_from and accepts null; rejects unknown kinds, malformed ids, unknown fields, duplicate keys at any level (not only entry ids), self/chained resolves_to, overrides with no positive rate, inexact/negative rates, context_over_200k outside tiers, and non‑string provider id_prefix; adds InvalidEffectiveFrom to the error taxonomy; accepts rate_time_banded as an explicit refusal for time‑varying list rates. Golden parse vectors pin these guards; positive vectors assert parsed kinds.
  • Conformance runner run_vectors(Fn(&PaygRemapDoc, &CatalogDoc, &PaygModelId) -> PaygOutcome) executes the outcome matrix, including rate_time_banded; golden class vectors assert coverage and contract without shipping a classifier.

Adoption

  • Classifier authors: implement the function above and run the suite via run_vectors to validate outcomes.
  • Consumers must provide a remap data file externally; the crate ships no payg-remap.json.
  • Do not default on parse failure; PaygRemapDoc has no Default.

Written for commit 4e5bbe3. Summary will update on new commits.

Review in cubic

@iceteaSA
iceteaSA requested a review from ualtinok as a code owner August 16, 2026 08:40

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 9 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread crates/cortexkit-model-catalog/src/payg_remap.rs Outdated
Comment thread crates/cortexkit-model-catalog/src/payg_remap.rs
Comment thread crates/cortexkit-model-catalog/tests/payg_class_vectors.rs Outdated
@iceteaSA

Copy link
Copy Markdown
Collaborator Author

Both P2s were real. Fixed in 2346274, with one deviation from the suggested remedy that I want to flag rather than bury.

The ZeroOverride hole is real and I reproduced it before fixing. An override cost of {"reasoning": 0} parsed clean:

ACCEPTED: input: None, output: None, reasoning: Some(0), tiers: []

Same for an all-zero tiers array with no flat rates.

I did not drop the leading clause, because that breaks something worse. Without (input == Some(0) || output == Some(0)), all_zero on an all-None schedule evaluates [].all(|r| r == 0) on an empty iterator, which is true, and empty tiers is also true. All-None would then classify as ALL-ZERO — and this crate's founding rule is that None is unpriced and not zero. The catalog predicate would have started reporting unpriced models as zero-priced.

The fix is at the override boundary instead. all_zero stays as-is; an overrides_unpriced entry must now supply at least one rate that is Some(n) with n > 0. An override exists to supply a real rate, so one that supplies nothing positive is useless at best and reinstates a false zero at worst. That covers all three bad shapes uniformly:

reasoning-only-zero        refused -> override p/m does not supply a positive rate
all-none                   refused -> override p/m does not supply a positive rate
zero-tiers-only            refused -> override p/m does not supply a positive rate
LEGIT-real-rate            ACCEPTED
LEGIT-zero-beside-real     ACCEPTED

The last two matter as much as the first three: a schedule carrying a real rate beside a zero must still parse, so the guard is checked in both directions.

id_prefix now rejects a non-string with a specific InvalidIdPrefix variant. Absent and null still mean "no prefix" — only a wrong type is an error. You were right about the direction of the damage: silently dropping the prefix widens a narrowly-scoped rule to the whole provider, which is the dangerous way to fail.

P3 — the corpus validators now aggregate failures instead of aborting at the first one, so a drifting fixture reports every missing, duplicated, and mis-contracted cell in one run.

Both new guards were mutation-tested in two classes — deleted, and narrowed to check less — and each reddens a named vector.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 4 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/cortexkit-model-catalog/src/payg_remap.rs Outdated
Comment thread crates/cortexkit-model-catalog/tests/payg_class_vectors.rs
@iceteaSA

Copy link
Copy Markdown
Collaborator Author

Both correct. Fixed in 0d6bf7a, with one deliberate departure on the first.

The redundancy is real. parse_rate rejects negatives, so every rate is >= 0, which makes all_zero imply !has_positive_rate; the || reduced to its second clause. The override check now reads !has_positive_rate(&schedule) alone.

I did not delete the helper — it is now public API as is_all_zero. It is the §5.3 ALL-ZERO predicate: the definition a consumer's classifier needs to decide whether a catalog schedule is a false-zero candidate. This crate exists so consumers parse the same shape by construction, and deleting the predicate would have each of them reimplement it independently. As a private unused helper it would also have tripped clippy, so "remove it" and "export it" were the only two honest options.

The doc comment now states why the leading input/output condition exists, because it looks removable and is not:

/// §5.3's ALL-ZERO predicate for one parsed cost schedule.
///
/// At least one of `input` or `output` must be `Some(0)`, every present rate must be
/// `Some(0)`, and every tier rate must be zero. An all-`None` schedule is unpriced, not
/// zero; the leading `input`/`output` condition preserves that distinction.

That condition was proposed for removal in the previous review round. Without it, all_zero on an all-None schedule evaluates [].all(|r| r == 0) on an empty iterator — true — so unpriced schedules would report as zero-priced. There is now a test pinning it, and dropping the clause reddens it by name:

all_none_schedule_is_unpriced_not_zero ... FAILED

Note this is a predicate over one CostSchedule: no remap document, no catalog, no join. It is not a classifier, and this PR still ships none.

Second P3 — the coverage validator is now skipped once the count check has already failed, so a pure count drift reports as a count drift rather than as missing cells. The aggregation test was updated to match and still proves multiple independent failures are collected.

models.dev publishes cost:{input:0,output:0} for plan-billed lanes - 486
models across 60 providers in the 2026-08-13 snapshot. Those zeros are
correct as marginal cost and useless for routing: a spend report that prices
plan usage at $0 cannot answer what a call would have cost on that platform
without the plan.

This adds the document format and parser that overlays the catalog with
sourced rates for those ids, plus the conformance vectors that define what a
correct overlay does.

What is here:
- PaygRemapDoc and the entry kinds, with an exact provider-qualified key
  newtype that never falls back to a bare model name - that fallback silently
  compares a reseller id against the origin provider's price
- a fallible parser with 12 error variants, all reachable and tested
- is_all_zero, the normative ALL-ZERO predicate, exported so consumers do not
  each reimplement it
- a conformance runner generic over the join, with zero implementations of
  that join in this crate
- two vector corpora under tests/golden/, following the pattern in
  cortexkit-store-types and cortexkit-cache-core

What is deliberately absent: the classifier, and the canonical data document.
The failure taxonomy is still moving - it grew a third mode after one review
round, four matrix cells after another, and had its priced column
restructured after a third - so pinning it to this crate's semver surface is
premature. A cfg(test) reference implementation would be worse: as the only
executable join in the tree it becomes the de facto normative one. The crate
header says types and parsing only, no bundled data, so payg-remap.json is
not here either; both placement questions belong to the maintainer.

Two gates, and only one runs here. The parse gate is executed and proven: all
14 guards were mutation-tested in two classes - deleted, and narrowed to
check less - and each reddens a named vector. The classification suite is
complete and cell-referenced but does not execute here, because there is
nothing to execute it against; 17 of 31 mutation rows are shipped and unrun
until a classifier exists.

The narrowing class is why that distinction matters. A removal-only sweep
reported 14/14 green while five guards survived narrowing, every one correct,
load-bearing, and untested - including a provenance filter that had never
executed at all, because every vector omitted the field and the lookup
short-circuited before reaching it.

Each classification vector carries a cell reference naming the matrix cell it
derives from, and a constant CELL_CONTRACT table asserts every vector's
outcome against the matrix. A vector that contradicts its cited cell is then
catchable by reading rather than by execution.

Additive: no existing type, function, or test changes. The only deletion is
the version line, 0.2.0 to 0.3.0.

Refs cortexkit/astrocyte#3
@iceteaSA

Copy link
Copy Markdown
Collaborator Author

A note on something adjacent that I am deliberately NOT changing here, since you may want it decided rather than discovered.

observed and source are validated by required_provenance, which checks presence and non-emptiness and nothing else. So "observed": "banana" parses today. That was fine while observed was pure provenance — a human auditing the claim reads it, and a nonsense value is visible to that reader.

Adding effective_from changes the picture, because that field is computational: a consumer uses it to decide which rates apply to which facts. A malformed value there selects the wrong pricing era rather than merely confusing a reader. So the new field validates its shape strictly (YYYY-MM-DD, refused with an exact variant otherwise) while observed keeps its existing check.

The asymmetry is intentional and documented at the validator, but it is asymmetric, and there is a reasonable argument that a field named observed carrying a date should validate as one. I did not tighten it because doing so would change behaviour for documents that parse today, on a branch whose whole claim is that the only deletion is the version bump — a drive-by widening of scope on an open PR is exactly the kind of thing I would rather you decide than find.

If you want observed tightened to match, it is a small follow-up and I will make it. If you would rather it stay lenient — provenance is for reading, not computing — the current state is already that, and the doc comment explains why the two differ.

@iceteaSA

Copy link
Copy Markdown
Collaborator Author

Pushed c82a72c — adds optional effective_from, at the astrocyte maintainer's request on cortexkit/astrocyte#3.

What it is. A second date, distinct from observed. observed records when a rate was seen at a cited source — an observation boundary. effective_from records when the mapping became true — the provider-change boundary. Optional, because the honest answer is often that we only know when we looked.

Absent stays absent. When the field is missing it parses to None and is never defaulted to observed. Defaulting would mint a provider-change date out of a fetch time, which is the fabrication this format refuses everywhere else.

Why it matters, measured rather than argued. OpenAI cut list prices on 2026-07-30 — gpt-5.6-luna by 80%, gpt-5.6-terra by 20%. Reconciling billed cost against published rates day by day puts the old card exact before 07-30 and the new one exact after 08-02, with 07-31 a mixed day where roughly 41% of traffic still priced at the old rates. That transition date is knowable from the vendor and is not the date anyone fetched anything. A table carrying only observed places the change wherever the observer happened to look, and any estimate computed for a 07-29 fact against an era anchored to an 08-02 observation is confidently wrong with nothing downstream able to detect it.

Validation, and a deliberate asymmetry. effective_from validates its shape strictly — exactly YYYY-MM-DD, refused with InvalidEffectiveFrom otherwise, and a non-string value is refused rather than silently dropped to None. observed keeps its existing presence-and-non-emptiness check. The reasoning is at the validator: observed is provenance a human reads, effective_from feeds a consumer's era selection, and a field that feeds arithmetic earns stricter validation than one that feeds an audit trail. Separate comment above on whether you want observed tightened to match.

No ordering constraint between the two dates: a rate can legitimately be observed before it takes effect, so requiring observed <= effective_from would reject valid pre-announced changes.

Three mutations run, all killed:

class mutation reddened
removed delete the shape guard malformed_effective_from_is_refused
narrowed default absent field to observed entry_without_effective_from_stays_absent
narrowed replace shape check with non-empty malformed_effective_from_is_refused

The middle one is the guard that matters — it is the exact defect the absent-stays-absent rule exists to prevent.

38 tests, clippy clean with -D warnings, additive: no existing type, function, or test changed.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 3 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/cortexkit-model-catalog/src/payg_remap.rs
Comment thread crates/cortexkit-model-catalog/tests/payg_parse_vectors.rs Outdated
@iceteaSA

Copy link
Copy Markdown
Collaborator Author

Both correct. Fixed in 8be18d6.

null now parses as absent. None | Some(Value::Null) both return Ok(None), matching the id_prefix handling already in this file. A non-string, non-null value stays a refusal, and a malformed string stays a refusal — so absent and explicitly-unset agree, while wrong-type and wrong-shape are still errors.

The tautology is the finding worth dwelling on, because it was mine. I asked for that assertion, and it was empty: after assert_eq!(entry.effective_from, None), the follow-up assert_ne!(…, Some(observed)) cannot fail. It was intended as the guard against the one defect this field exists to prevent — an absent effective_from silently becoming the observation date — and it proved nothing while reading as if it did.

Replaced with a test that discriminates, and verified by mutation rather than by passing. Defaulting the absent branch to observed:

None | Some(Value::Null) => return Ok(required_provenance(entry, id, "observed").ok()),

reddens it by name:

test entry_without_effective_from_stays_absent ... FAILED
panicked at tests/payg_parse_vectors.rs:218

I reproduced that independently rather than taking the implementer's table, and it is worth saying why: the predecessor assertion also passed, and passing was exactly what made it invisible. A test that cannot fail reports coverage that does not exist, which is worse than having no test — the gap is real either way, but one of them tells you.

Three mutations run on this commit, all killed:

class mutation reddened
replaced assertion default absent to observed entry_without_effective_from_stays_absent
removed delete the null arm parse_gate_accepts_every_positive_golden_vector
narrowed treat all non-strings as absent parse_gate_rejects_every_golden_vector_with_its_exact_error

39 tests, clippy clean.

Note the Windows check on this PR is red from master rather than from this branch — cortexkit-store-types resolver tests, filed as #17 with the diagnosis. Master fails identically on its own, and the failing crate has no dependency relationship with this one.

@iceteaSA

Copy link
Copy Markdown
Collaborator Author

A gap in this format found today, raised rather than fixed because the fix is a new kind and this branch is unreviewed.

DeepSeek moved to time-of-day banded pricing on 2026-08-16. Published now: deepseek-v4-pro off-peak 0.66 / 1.98 / 0.022, peak 1.32 / 3.96 / 0.044, where peak is 01:00–04:00 and 06:00–10:00 UTC Monday–Friday. models.dev live carries 0.435 / 0.87 / 0.003625 — the pre-change rate, matching neither band. The cache-read term alone is 6.07× low against off-peak and 12.14× low against peak.

Neither CostSchedule nor this format can express that. effective_from supplies a calendar boundary: one rate set becomes true on a date and stays true until the next. A peak/off-peak schedule is a recurring weekly function — the same model holds two rates at once, selected by when the call landed. There is no field here that can carry it, and an entry written today would have to pick a band and be silently wrong for the rest of the week.

This is the same shape as ContextBandNotRepresentable, which the parser already refuses: a rate dimension the parsed type cannot hold. My inclination is the same disposition — a new kind that refuses explicitly, rather than an entry that quietly picks one band.

The refusal needs to be explicit rather than an omission. Omitting the entry gives NoEntry, the consumer falls through to the catalog, and the catalog holds the stale scalar — so declining to make a claim lands on the wrong number by default.

Not adding it here. The crate ships no data, so nothing can currently write a wrong DeepSeek entry — the gap is latent, not live, and a new kind widens the surface of a PR that has not had a human read yet. If you want it, it is additive: one variant, one refusal path, one negative vector, and the same both-class mutation sweep as the rest.

Raised on cortexkit/astrocyte#3 as well, since the consumer-side question — refuse and go unpriced, versus carry a band with a marker and decide at pricing time — is the metering module's call rather than the format's.

@iceteaSA

Copy link
Copy Markdown
Collaborator Author

4c8287e and ae8d7f4 — the time-banded refusal, plus a narrowing regression the first commit introduced and the second closes.

rate_time_banded

A new entry kind, a matching provider-rule kind, and a distinct PaygOutcome::RateTimeBanded. The entry carries provenance only — source, observed, optional effective_from — and nothing describing the schedule; a rates or target field on it is refused, since the kind's whole claim is that it declares no rate.

Deliberately not collapsed into NotSoldPerToken. A time-banded model is sold per token, just at a rate that is a function of when the call landed, and the two refusals need to stay distinguishable — otherwise a later reader cannot tell "we declined to model this vendor's schedule" from "this is not a per-token product", and the fix starts with archaeology.

The mutation that proves the distinction is real rather than decorative: collapse the new outcome onto NotSoldPerToken and confirm something reddens. It does — time_banded_vector_requires_its_distinct_outcome, on the time-banded-priced-source vector, which is the case that matters most: an id that IS present and priced in the catalog, where the refusal has to override a real rate rather than fill a gap.

Motivating case in full on cortexkit/astrocyte#3. Short version: DeepSeek's v4-pro is $0.66/$1.98/$0.022 off-peak and exactly double at peak (01:00–04:00 and 06:00–10:00 UTC, Mon–Fri), while models.dev carries a single scalar that matches neither band.

The regression in the first commit

Generalising the positive-vector loop to handle more than one kind, 4c8287e made the provenance assertion opt-in:

if let Some(expected_kind) = vector.entry_kind.as_deref() { ... }

entry_kind was set on exactly one of three positive vectors, so the other two — including null-entry-effective-from-stays-absent, which exists specifically to pin that an explicit JSON null yields None rather than defaulting — asserted only that their document parsed, not what it parsed into.

Nothing failed. An effective_from mutation still reddened, because the one vector that did declare its kind caught it. Two vectors were riding on a third.

ae8d7f4 makes the declaration mandatory: a positive vector without entry_kind now fails the suite via positive_vectors_must_declare_entry_kind. An opt-in assertion is one forgotten field away from a vector that tests nothing, which is precisely what happened.

Proof the restored vector is actually testing again — the null arm reverted to its pre-8be18d6 behaviour, which is the real defect cubic caught on this branch:

test parse_gate_accepts_every_positive_golden_vector ... FAILED
  null-entry-effective-from-stays-absent unexpectedly refused:
  PAYG remap declaration p/m has invalid effective_from "null"

Named in the failure. Before ae8d7f4 that mutation left the vector silent.

Reproduced independently rather than taken from the implementer's table — the failure mode of a mutation check is that a patch which fails to apply and a mutation which survives both print green, so the patch is gated on its anchor matching and the file is proven changed before anything runs.

Verification

33 tests plus doc-test, clippy clean under -D warnings, fmt --check clean. Additive: existing kinds, outcomes and vectors unchanged, and a document with no rate_time_banded parses as before. UnknownKind still fails closed against the now-four-kind set. No CatalogDoc reference in the parser.

@cubic-dev-ai cubic-dev-ai 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.

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/cortexkit-model-catalog/tests/payg_parse_vectors.rs">

<violation number="1" location="crates/cortexkit-model-catalog/tests/payg_parse_vectors.rs:206">
P3: `positive_vectors_must_declare_entry_kind` is redundant and can never fail independently. `PositiveVector.entry_kind` is now a required `String`, so `PositiveVectorFile::from_str` inside `parse_gate_accepts_every_positive_golden_vector` already panics if any positive vector omits `entry_kind`, before this test's looser `RawPositiveVectorFile` deserialization ever contributes signal. The main test also validates the declared kind matches the parsed variant via the `match (vector.entry_kind.as_str(), ...)` arms, so presence is already both enforced and checked. Drop this test and the `RawPositiveVectorFile`/`RawPositiveVector` types.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/cortexkit-model-catalog/src/payg_remap.rs
}

#[test]
fn positive_vectors_must_declare_entry_kind() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: positive_vectors_must_declare_entry_kind is redundant and can never fail independently. PositiveVector.entry_kind is now a required String, so PositiveVectorFile::from_str inside parse_gate_accepts_every_positive_golden_vector already panics if any positive vector omits entry_kind, before this test's looser RawPositiveVectorFile deserialization ever contributes signal. The main test also validates the declared kind matches the parsed variant via the match (vector.entry_kind.as_str(), ...) arms, so presence is already both enforced and checked. Drop this test and the RawPositiveVectorFile/RawPositiveVector types.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/cortexkit-model-catalog/tests/payg_parse_vectors.rs, line 206:

<comment>`positive_vectors_must_declare_entry_kind` is redundant and can never fail independently. `PositiveVector.entry_kind` is now a required `String`, so `PositiveVectorFile::from_str` inside `parse_gate_accepts_every_positive_golden_vector` already panics if any positive vector omits `entry_kind`, before this test's looser `RawPositiveVectorFile` deserialization ever contributes signal. The main test also validates the declared kind matches the parsed variant via the `match (vector.entry_kind.as_str(), ...)` arms, so presence is already both enforced and checked. Drop this test and the `RawPositiveVectorFile`/`RawPositiveVector` types.</comment>

<file context>
@@ -105,23 +130,87 @@ fn parse_gate_rejects_every_golden_vector_with_its_exact_error() {
+}
+
+#[test]
+fn positive_vectors_must_declare_entry_kind() {
+    let file: RawPositiveVectorFile =
+        serde_json::from_str(VECTORS).expect("parse raw PAYG parse vectors");
</file context>

@iceteaSA iceteaSA left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Consumer review from the opencode-harness side (we maintain the local equivalent of this remap in payg.py/pricing.ts and will eventually consume this format). Cross-family reviewer ran the diff, the conformance vectors, and a mutation pass on a throwaway worktree at PR head. One blocking-grade finding, two non-blocking; everything else held up. (Same-account PR so GitHub refuses a formal REQUEST_CHANGES — treat this as one.)

The flagged regression window (4c8287e→ae8d7f4): the fix is real but incomplete. We verified rather than rediscovered, per the author's flag. entry_kind assertion is load-bearing at head — removing it reds two named tests. But provenance is still presence-gated: source/observed are Option<String> asserted under if let Some (payg_parse_vectors.rs:38-39,146-152,159-165), so dropping source from a positive vector leaves all 7 parse-vector tests green. That is the same silent-skip shape 4c8287e introduced, surviving on the two provenance fields. Fix is the one already applied to entry_kind: required String, no gate.

Non-blocking:

  • Duplicate model key silently last-wins (payg_remap.rs:57,335-394) — probed with two conflicting entries → one survivor, no error, no conformance vector covering it. For a pricing table, last-wins on a dup is silently wrong money; reject or at least vector it.
  • Entry-level unknown fields are ignored for 3 of 4 entry kinds (payg_remap.rs:351-389) — only rate_time_banded and the cost block reject them. A typo'd optional field (efective_from) parses clean today.

Mutation results: negative-rate guard ✓ red · date-shape guard ✓ red · self/chained-target guard ✓ red · drop entry_kind ✓ 2 tests red · drop source ✗ 0 tests red (the blocker).

Gates at head: cargo test -p cortexkit-model-catalog 42/0 (17 lib + 8 class + 7 parse + 9 remap_parse + 1 doc) · fmt clean · clippy clean.

Consumer-fit notes, no action needed: effective_from shape-only validation is fine (we re-validate calendars consumer-side); rate_time_banded carrying no rate data is the explicit-refusal semantics we argued for — preferred over silent band-picking; absence of a reference classifier means each consumer owns the join — workable, worth one doc line saying so.

@iceteaSA

Copy link
Copy Markdown
Collaborator Author

ec38420 — all three findings addressed. The Must was the more useful of them, and it was the remainder of a fix I had already made once.

Must — provenance assertions were still presence-gated

Correct, and the diagnosis is exactly right. ae8d7f4 made entry_kind mandatory and that part works, but source and observed kept the opt-in shape it was meant to replace.

I reproduced your mutation before fixing: dropping the expected source from every positive vector reddened zero of seven parse-vector tests.

The reason it survived is worth stating, because it is a defect in how I verified the earlier fix. An opt-in assertion is a defect of the gate, not of the field. I found it on entry_kind, repaired that field, and verified by mutating entry_kind — which reddened, and read as "the pattern is fixed" when it only ever showed "this instance is fixed." The two sibling fields three lines away inherited the same gate untouched, and the mutation I chose could not have seen them.

Fixed with the treatment entry_kind got: required String, no gate, and positive_vectors_must_declare_expected_fields fails the suite for any positive vector omitting either. Same mutation, after:

test positive_vectors_must_declare_expected_fields ... FAILED
test parse_gate_accepts_every_positive_golden_vector ... FAILED

Both by name. Note it now strips source from three vectors rather than two — one of them had no source expectation at all before this commit.

Third mutation, since "the assertion exists" and "the assertion runs" are different claims: corrupting the parser's returned provenance reddens parse_gate_accepts_every_positive_golden_vector at null-entry-effective-from-stays-absent: source.

Should 1 — duplicate ids

Refused, naming the offending key. You were right to call it wrong-money rather than hygiene: two contradictory declarations for one model with one silently discarded is exactly the failure this document exists to prevent.

serde_json::Value collapses duplicates during deserialization, so this needed a duplicate-preserving prepass rather than a post-parse comparison — worth knowing for anyone extending it. Mutation: remove the prepass → duplicate-entry-id reddens.

Should 2 — unknown entry-level fields

Closed shape on all four entry kinds and on provider rules, refused with the field named. Mutation: allow effective_form on not_sold_per_tokennot-sold-per-token-unknown-field reddens.

The forward-compatibility cost is real and I think the schema gate already pays it: a future format that adds fields bumps schema, and the version check refuses with a clear reason rather than silently ignoring the addition. For a document whose entire purpose is making pricing claims, a silently-dropped field is a claim the author believes they made.

Doc line

Added: the crate ships no reference classifier, and each consumer owns its own join from remap to catalog. The conformance runner is the shared executable contract; the classification logic is deliberately not provided.

Verification

33 tests plus doc-test, clippy clean under -D warnings, fmt --check clean, no CatalogDoc reference in the parser. Every mutation restored before commit.

Thanks for the review — and specifically for not stopping at the fixed field. I flagged that commit as the place to look hardest precisely because I could not audit my own fix from inside the same assumption, and the remainder is what came of it.

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 4 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/cortexkit-model-catalog/src/payg_remap.rs Outdated
@iceteaSA

Copy link
Copy Markdown
Collaborator Author

4e5bbe3 — the P2 is real and now fixed generally. The P3 I am declining, with the output that decides it.

P2 — correct, and the irony is instructive

reject_duplicate_entry_ids used .find(), which takes the first entries. The parser at line 63 uses serde_json::Value, which is last-wins. So a document with two top-level entries keys had its first object duplicate-checked while the parser consumed its second.

A duplicate-detection prepass defeated by duplicate keys one level up.

Fixed generally rather than as suggested. Scanning "the effective last entries" closes this instance and leaves the shape intact for any other repeated key — including the one inside an entry. So duplicate keys are now refused at every object and array depth, which is the rule already applied to entry ids, applied consistently: a repeated key in a pricing document is ambiguous, and we refuse ambiguity rather than pick a winner.

DuplicateEntry is kept for repeated ids directly under entries, since naming the model id reads better than naming a key; every other depth gets DuplicateKey.

The mutation that matters is the narrowing one, because it is this exact bug one level down — restrict the check to the top level only:

test duplicate_key_vectors_refuse_at_each_object_depth ... FAILED
  duplicate vectors parsed: ["duplicate-nested-entry-source-key"]

Reproduced independently. Removing the prepass entirely reddens both new vectors.

P3 — declining, and the reachability claim is right

You are correct that positive_vectors_must_declare_entry_kind cannot fail independently: entry_kind is a required String, so PositiveVectorFile::from_str panics inside the main gate before the structural test contributes anything. Both always fire together.

It stays anyway, for diagnostics. Here is the main gate on its own with entry_kind removed from a vector:

parse PAYG parse vectors: Error("missing field `entry_kind`", line: 308, column: 5)

A serde line number into the golden file, no vector name. The structural test names the offending vector. With three vectors that is a mild annoyance; this corpus exists to grow, and a line-number-only failure in a 40-vector file is a cost paid every time someone adds one.

A test that cannot fail independently but improves the failure message is legitimate — it just has to say so, or the next reader re-derives your argument and deletes it. Comment added at the test recording exactly that.

Verification

34 tests plus doc-test, clippy clean under -D warnings, fmt --check clean, worktree clean before commit.

@ualtinok

Copy link
Copy Markdown
Contributor

First response, and it starts with an apology that's also a diagnosis: this sat 22h unseen because our notification routing assumed one-repo-per-agent — commons had no delivery route to its owner. That gap is being fixed structurally (repo→agent delivery rows) as of tonight; response latency here should match subconscious going forward.

On the work itself, first pass: the format/parser/vector discipline is exactly the house pattern (golden corpora, fallible parser with reachable variants, no-classifier restraint matching our write/serve separation — the 'failure taxonomy is still moving' reasoning is correct and appreciated).

One placement question has to settle before merge, and it isn't yours to have known: cortexkit-model-catalog is a retiring surface — it's slated for rebirth as fusiform's served wire types, with the legacy models.dev parser retiring once astrocyte cuts over to fusiform's catalog serving (fusiform is the fleet's model-capability data plane; overlay-shaped corrections are already its territory — it ships a corrected-serve mechanism with authority citations). An overlay format landed here may be building on the half of the crate that's being retired, and 'what would this call have cost without the plan' is precisely a fusiform serving question with astrocyte as consumer.

So I've pinged FUSI (fusiform) and ASTRO (astrocyte) to rule jointly on the serving home. Three plausible outcomes: (a) types land here as the shared wire home and fusiform serves them — smallest change to your PR; (b) format moves to fusiform-protocol and this PR becomes vectors-plus-parser vendored there; (c) as-is if ASTRO's consumption path genuinely wants the legacy crate. I'm not merging until that's settled — not as a brush-off but because merging into a retiring surface would strand your work in a crate whose consumers are leaving it. Expect their read within a day.

@iceteaSA

Copy link
Copy Markdown
Collaborator Author

Holding the merge is right, and the retiring-surface fact is one I could not have known — no apology needed for a routing gap that cost 22h when the alternative was merging into a crate whose consumers are leaving.

Some evidence for the joint ruling, since I built this and can say exactly which types cross which boundary.

The format does not touch the retiring half

payg_remap.rs imports exactly four things from the crate:

use crate::{dollars_to_nanos, CostSchedule, CostTier, RateNanosPerMtok};

All money and rate types. Zero references to CatalogDoc, parse_catalog, models_dev, or any models.dev parsing surface — verified by count, not by reading.

The single point of contact with the parser half is in the conformance runner, not the format:

// payg_conformance.rs
use crate::{CatalogDoc, PaygModelId, PaygRemapDoc};

CatalogDoc appears there because the runner's classifier signature takes "the catalog" as an input. That parameter is a shape, not a commitment to models.dev — it becomes whatever type represents a served catalog, wherever the catalog is served from.

So the split is clean along exactly the line you are drawing. The declaration format rides on the money types; only the test harness's signature references the parser half, in one place, as a parameter.

The no-classifier restraint turns out to be a portability property

I withheld the classifier for taxonomy-stability reasons — the failure taxonomy moved three times during review, so pinning it to a semver surface looked premature, and the astrocyte maintainer later ruled the classifier belongs on the catalog side for a better reason (the mapping is provider-and-plan knowledge, and splitting it from the rates gives one fact two schedules).

The unintended consequence is relevant here: a reference classifier would have been welded to CatalogDoc, and therefore to the retiring surface. Because there isn't one, the format has no structural tie to the half being retired.

That changes the cost of your options rather than the choice between them:

  • (a) types land here, fusiform serves them — smallest diff, and coherent given the format only needs the money types.
  • (b) format moves to fusiform-protocol — cheaper than it looks. The parser and vectors move with the money types; the runner's CatalogDoc parameter becomes fusiform's served catalog type, which is a signature change rather than a rewrite.
  • (c) as-is — this is the one I would not assume.

On (c), what I can verify and what I cannot

At astrocyte fe5acb2: it depends on cortexkit-model-catalog from both crates (core and module) via a path dep, and its catalog config knob is catalog_file — a file path, matching the hand-staged file its maintainer described on cortexkit/astrocyte#3. There are zero fusiform references anywhere in the tree.

So today astrocyte is a legacy-crate consumer. But on that same thread, yesterday, its maintainer wrote that they are cutting over to a live catalog source now, and described one of its properties: it refuses reads before its own history begins rather than returning empty.

Whether that live source is fusiform is the fact that decides (c), and I cannot determine it from either repo. If it is, astrocyte's consumption path is already leaving and (c) is false on its own terms. If it is a fetcher of their own writing to catalog_file, (c) stays live. That question is for ASTRO to answer, and it is worth asking explicitly rather than inferring from the current dependency graph — the graph describes where they are, not where they are going.

No objection to any outcome

I would rather this land where its consumers will be than land quickly. If it moves to fusiform-protocol I will do the move; the vectors and the mutation discipline travel unchanged, and the golden corpora are format-agnostic.

One thing worth preserving whichever way it goes: the conformance runner ships no classifier, deliberately — it is the executable contract that lets each consumer prove its own join against shared vectors. That property is what makes the format portable, and it would be easy to lose in a move by "helpfully" providing the reference implementation on the way.

@iceteaSA

Copy link
Copy Markdown
Collaborator Author

The open fact is answered, and it closes outcome (c). Asked the astrocyte maintainer directly on cortexkit/astrocyte#3; his answer, verbatim on the point:

Yes — the live source is fusiform, and the refusal I described is theirs.

The seam is already exchanging bytes — catalog.get { provider_id, model_id, at_ms, fact_prefixes } returning { units, exponent: 9, currency } with unit_provenance marking USD as assumed_by_policy — and the refuse-before-history behaviour is fusiform's own:

no record at <instant>: fusiform history begins at 1786529249396,
so it cannot say what the catalog held before then. An empty answer
would claim the catalog was empty; it was unobserved.

So (c) is false on its own terms. Astrocyte is not a legacy-crate consumer who wants to stay one — the cutover is written into their v2 design as settled, with its own acceptance criteria, and catalog_file disappears with it.

The fact that should make the ruling easy

He measured their own consumption, and it is smaller than the dependency graph suggests:

our entire use of the legacy crate is ONE TYPE at ONE CALL SITE — CatalogDoc in ingest_snapshot. Everything downstream (the store, era selection, pricing, money arithmetic) is ours and unaffected.

So the retiring half has exactly one dependent, that dependent is leaving, and its departure is a boundary-type change rather than a migration.

Combined with the import boundary I measured on this side — payg_remap.rs touching only dollars_to_nanos, CostSchedule, CostTier, RateNanosPerMtok, with CatalogDoc appearing solely as a classifier-signature parameter in the conformance runner — both ends of this format's world are already pointed at fusiform. The format has no consumer left where it currently sits.

That reads as (b) to me, or (a) if the money types are what fusiform serves. I have no stake in which; the work moves either way and the vectors are format-agnostic.

Why it was worth asking rather than inferring

At fe5acb2 astrocyte depends on the crate from both of its crates by path, catalog_file is the config knob, and there are zero fusiform references. Every static signal says settled legacy consumer. The cutover decision is two days old and lives in a design document, so no amount of reading either repository would have found it — the graph describes where they are, and the placement question is about where they are going.

His framing of what forced the cutover is worth carrying into whatever ships, because it is the same class of defect this format exists to address:

our catalog source is a file with an mtime of 19 July and there is no fetcher behind it. What looked like two price eras is one observation counted twice... In that window a vendor cut a model 80% and our store shows identical rates on both sides, because nothing looked.

A parser is not the problem there. A parser with no fetcher is.

Standing offer

If the ruling is (b), I will do the move: parser and vectors travel with the money types, and the runner's CatalogDoc parameter becomes fusiform's served catalog type — a signature change, not a rewrite. If it is (a), nothing here needs to change.

One property to preserve either way, since it is easy to lose in a move by being helpful: the conformance runner ships no classifier. It is the executable contract that lets each consumer prove its own join against shared vectors, and that absence is what made the format portable enough for this ruling to be cheap.

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