Skip to content

compass: the cost backend interface, its breakdown and its ladder (W1.5, #23) - #58

Merged
jgong5 merged 3 commits into
feature/atomcompass_newfrom
compass/w1.5-cost-backend
Sep 21, 2026
Merged

jgong5 merged 3 commits into
feature/atomcompass_newfrom
compass/w1.5-cost-backend

Conversation

@jgong5

@jgong5 jgong5 commented Sep 21, 2026 •

Copy link
Copy Markdown
Owner

Closes #23 (the main agent closes it at landing, with the handoff comment).

Dev record. Four things, in atom/compass/backends/: the backend interface, the step cost it returns, the vocabulary every number in that cost is labelled with, and the order sources are consulted in. Nothing concrete is built — the only backend in the diff is a nine-line stub inside a test, there to exercise the interface.

Two review rounds landed as 1e03d3979 and 344c19e89. The sections below describe the current state; What review changed at the end records what moved and why.

The interface, as W1.6 will consume it

class CostBackend(abc.ABC):
    @property
    @abc.abstractmethod
    def tier(self) -> Tier: ...                        # "0" | "a" | "b"

    @abc.abstractmethod
    def estimate(self, batch_view: Any) -> StepCost: ...

    @abc.abstractmethod
    def describe(self) -> str: ...

estimate returns a StepCost or raises CostRefused. It never returns a float.

batch_view is deliberately untyped here. It is a projection the caller prepares, and its shape belongs to the task that builds the runner, not to this one; defining it here would mean guessing at fields the runner has not settled. What this package does commit to is that the projection is plain data: test_the_package_imports_nothing_from_the_engine parses every module under atom/compass/backends/, recursively, and asserts that the only atom.* imports are its own, so the package stays importable and testable without an engine or a driver.

The obligation that travels with the projection, written into base.py where its author will read it rather than only here: the no-engine-imports property belongs to a package, not to a type. That test reads the files under this package and nothing else. A projection defined here is covered the day it is added; a projection defined anywhere else is covered by nobody until its own package's tests assert the same thing over its own sources. A projection that reaches an engine type for one field can only be built where the engine imports, which defeats the reason it exists.

Decomposition, enforced by construction

StepCost has one field, terms. There is no constructor that takes a total, and the check runs on every read, not only at construction:

@property
def seconds(self) -> float:
    return fold_seconds(term.seconds for term in _checked(self.terms))

rows() is guarded the same way, and __init_subclass__ refuses a subclass that redefines any public reader — the set is read off the class (vars(StepCost) plus its annotations), not listed, so a reader added later is covered the day it is added and not the day somebody remembers to extend a tuple. Seven today: seconds, terms, rows, is_refused, refusals, refused_seconds, seconds_by_species. is_refused is the dangerous one: hiding it makes a run report no refused steps over steps that did refuse, with no bypass, through the public API, at the layer that reads that fraction to decide whether the run is evidence.

Three refusals sit in the constructors rather than in a convention:

  • An empty breakdown is refused. The failure behind it is the mean of an empty sample, which is not an error and is not approximately zero — it is exactly 0.0, and it reads as a step that took no time rather than as a model that was missing.
  • Duplicate term names are refused. A breakdown is read and grouped by name.
  • A refused term priced at zero is refused. A free stand-in removes the step from the schedule while still reporting it as priced.

Negative and non-finite durations are refused too. A zero-valued term is allowed when nothing refused — "no collective on this step" is a fact about the step, and suppressing it would remove a row a reader expects to find. The same empty-sample shape is refused one layer up: ProvenanceMix will not divide by an empty run (below).

Bypass routes, re-run at 344c19e89:

Route Result
dataclasses.replace(step, terms=()) ValueError (revalidates)
object.__setattr__ then read ValueError
__dict__ write then read ValueError
__new__ + setattr then read ValueError
pickle of a hacked object ValueError
subclass shadowing any of the seven readers TypeError at class definition, 7 of 7
rows() on a hacked object ValueError

copy and pickle of a valid step round-trip and fold identically. The two mechanisms are needed because the route classes are disjoint: a guard inside seconds cannot observe a subclass that replaces seconds, since the parent's reader never runs.

Fold order. Every addition of seconds goes through fold_seconds — a left fold from 0.0 in the order given — with fold_step as its one-step form for running totals. By AST enumeration the package contains exactly five Add nodes: one in fold_seconds, three integer counters (steps, refused_steps, a reason count) and one tuple concatenation. rows() emits in stored order, so a reader with an artifact re-folds the rows and gets the total bit for bit. The sharp test: 0.1 + 0.2 + 0.3 is 0.6000000000000001 and the reverse order is 0.6.

Grouping is a different sum, and says so. seconds_by_species() folds each group over its own terms in stored order; grouping re-associates, so folding the group totals is not bit-equal to seconds. Terms of 0.1, 0.2 and 0.15 whose first and third share a species fold flat to 0.45000000000000007 and grouped to 0.45, and a test pins exactly that. The mixture is for reporting where the time went; rows() is the view a total is checked against.

The provenance vocabulary, and where each term came from

Five species, taken from the design's own glossary rather than invented:

Species Meaning
ANALYTICAL computed without measuring the subject
MEASURED the unit itself was timed
FITTED a form chosen, coefficients regressed over measured steps
INTERPOLATED no form assumed, nearby measurements looked up
EXTRAPOLATED asked outside the measured range

Two rules from the same source are carried in the shape rather than in prose. "Priced" is not a species, so there is no PRICED: a price is a measurement of a smaller unit, and the unit goes in Provenance.detail — measured (op-level) against measured (step-level). And reading a measurement back out of a file is still MEASURED; what a lookup changes is whether the key matched exactly, matched the nearest neighbour, or sat outside the measured range at all.

An unlabelled cost cannot be built. Provenance has no default species and rejects a species-shaped string; CostTerm has no default provenance.

refused is not a sixth species. Provenance carries refusals — every refusal collected on the way to this answer, earliest first — and source, the rung that produced it:

refused(price_list: unpriced leaf; nearest_key: outside the range) -> analytical (roofline) via analytic_law

A plain answer renders measured (op-level) via price_list. That rendering is the one thing the design left to the implementation.

The ladder, and how a fall-through is made observable

Resolver holds a fixed sequence of CostSource rungs and takes the first answer. A rung answers with a CostTerm, declines with self.refuse(reason) — a helper on the base class, so a source cannot misattribute a refusal to a neighbour — or, if it is itself a ladder, exhausts.

There is no path through Resolver.resolve that yields an unannotated or unattributed answer:

  • every refusal collected on the way down is stamped onto the answer's provenance, earliest first, so a middle rung's reason reaches the artifact and the reason counts;
  • the answering rung's name is stamped on every answer, fall-through or not, so a row can never render as -> analytical with nothing saying what produced it;
  • Resolution.declined holds every refusal this ladder collected, and fell_through is the one-bit version.

Composition works in both directions. On the answer path, a rung that is itself a ladder composes: outer refusals go in front of inner ones so the earliest is genuinely first, and the name reads outer/inner. Measured at four levels with two declines at the bottom: five refusals chronologically ordered, name lay2/lay3/lay4/a4, five distinct reason pairs.

On the exhaustion path, an inner CostRefused is caught and treated as a decline: its refusals join this ladder's and the next rung is consulted. Letting it propagate discarded the outer ladder's refusals and skipped every rung below the composed one — a run refusing where it had an answer available and unable to say what it passed over. The inner pairs stay keyed by the inner rungs that actually declined rather than being flattened onto the wrapper, so the reason counts still name something measurable. A source that raises with an empty list is attributed to the rung itself rather than vanishing.

When no rung answers, CostRefused carries the whole list, inner ladders' included — one run names every gap, instead of one gap per run. What the caller does next is the caller's policy, and this module has no opinion beyond refusing to invent the number.

What a refusal carries, and the three ways it is counted

A Refusal is (source, reason), both required and both non-blank — half a refusal cannot be acted on and cannot be grouped. ProvenanceMix accumulates a run and reports:

Reading Field
how many refused_steps
fraction of steps refused_step_fraction
fraction of predicted seconds refused_second_fraction
distinct reasons with counts reasons(), keyed (source, reason)
the mixture itself seconds_by_species()
nothing recorded yet is_empty

Both fractions are reported because they answer different questions: a run can refuse 2% of its steps and 40% of its time.

An empty run has no fractions and says so. Both properties refuse rather than returning 0.0, because "refused nothing" and "recorded nothing" would otherwise be the same number at exactly the layer that reads it — and of the two readings, the wrong one is the reassuring one. The denominators are separate: a run whose steps are all free keeps its step fraction, which is answerable, and refuses only the seconds fraction.

What the design did not cover, and what I decided

  1. Is a decline always a refusal? The design separates "refuse rather than fall back" from ordinary resolution but never draws the line. I made every decline a refusal, including an inner ladder's exhaustion. A quiet "not applicable" outcome beside a loud one reintroduces the silent fall-through this exists to prevent, and needs a rule for which declines are quiet that nothing in the design supplies.
  2. Ordering when several rungs decline, including across a composed ladder. The design says "the reason", singular. All of them are kept, earliest first; Provenance.refusal is the earliest, which is the one an operator has to close.
  3. Terms are flat, not nested. A hierarchy is spelled in the name (attention.decode). Consumers group and sum; none walks a tree.
  4. The rendering of a provenance, and the outer/inner composition of the answering name.
  5. CostRefused is raised, not returned. A refusal is a value (Refusal) everywhere it is a result; the exception exists only for "nothing could answer". Within a ladder that exception is caught and demoted back to values, so it crosses exactly one boundary — the backend's.
  6. An empty mix refuses rather than returning None. A sentinel would be silently formattable into an artifact; a refusal is not.
  7. The shadow set is derived, not enumerated. A listed set is correct on the day it is written.

Surprises

  • estimate's return type is the whole enforcement. Once StepCost cannot be built without labelled parts, there is nothing left for the interface to police. The abstract method got smaller as the value type got stricter.
  • The same defect arrived three times by three routes — the mean of an empty list inside a step, the fraction over an empty run, and a subclass hiding is_refused. Refusing a shape in a constructor does not make it refused everywhere; it has to be refused at each place it can be produced, which is why the shadow set is now derived rather than listed.
  • "First refusal wins" was right for a flat ladder and backwards for a composed one. Keeping the existing refusal and discarding the incoming one reads as conservative and is exactly wrong once a rung can be a ladder.
  • The exhaustion path had no correct implementation at all. Propagating discards the outer chain and skips lower rungs; catching and re-refusing flattens the inner pairs onto the wrapper and makes the reason counts name something nobody can measure. Catching and extending is the only shape that keeps both properties, and I did not see it until the composed case was written down.
  • test_the_package_imports_nothing_from_the_engine is parametrised over the package's own files, so the CPU pass count moves by one for each module W1.6 adds under backends/. Intended, but the total is not a constant across this wave.

What review changed

Round Finding Fix
1 Blocking — ProvenanceMix returned 0.0 for both refused fractions on an empty run Both refuse an empty denominator, separately; is_empty added
1 Blocking — Provenance.after dropped the outer refusal in a composed ladder Replaced by resolved(source, declined), which prepends; nothing is discarded
1 Middle-rung refusals never reached the artifact or the mix Provenance carries the whole chain; reasons() counts every rung
1 The artifact could fail to name the rung that answered The resolver stamps the answering name on every answer
1 A subclass overriding seconds drifts with no bypass __init_subclass__ plus a re-check inside seconds and rows
1 Correction to this body — five additions sat outside fold_seconds; the property held by coincidence Routed through fold_seconds/fold_step; five Add nodes total, four of them integer or tuple
2 Blocking — no correct way to write a resolver-backed rung: an exhausted inner ladder escaped, discarding the outer chain and skipping lower rungs The rung call is wrapped; an inner exhaustion extends this ladder's refusals and the next rung is consulted, inner pairs kept keyed by the inner rungs
2 The shadow set omitted is_refused, refusals, refused_seconds, seconds_by_species Derived from the class instead of listed; 7 of 7 refused
2 Grouping re-associates, so the mixture is not a second total Stated as the method's guarantee, with a test pinning a divergent case
2 The W1.6 note was satisfiable vacuously and the glob was non-recursive Glob made recursive; the note rewritten as an obligation on whoever defines the projection, in base.py
2 4084 was ambiguous between this branch and the integration head Every figure below names its commit; four trees measured

Left undone

  • No concrete backend, no batch-view type. Both belong to W1.6 and the runner task. This PR is the interface only.
  • ProvenanceMix implements no threshold. It reports the fractions; nothing here decides what makes a run admissible, and nothing here implements the mark-or-abort switch. Both are the run's policy.
  • No artifact serialisation. rows(), seconds_by_species() and reasons() are shaped for a writer; the writer is not in this diff.
  • Tier is declared, not checked. Nothing verifies that a backend calling itself op-level actually resolves at op level. That check needs artifacts that do not exist yet.

Gates

All four trees run on node 18 in xiaobizh_n18_cpu, against git archive snapshots (never rsync), md5-verified host → node → container, with PYTHONPATH asserted by the gate and pytest's own rc captured before any pipe. Every figure names its commit, because 4084 is now the total of two different trees.

Tree Commit Passed Skipped xfailed GATE_CPU_RC GPU tier
This branch's base c19710bcd 4030 149 3 0 not required
Integration head (CA-1 landed) f67618eb9 4084 149 3 0 not required
This branch 344c19e89 4098 149 3 0 not required
Merge of the two 7b6abcfd1 4152 149 3 0 not required

Arithmetic, both bases: 4030 + 68 = 4098, and 4084 + 68 = 4152. All four runs read 149 skipped and 3 xfailed, so the known one-in-four 4029/150 flake did not appear and no re-run was needed. The 68 new cases decompose 15 + 29 + 15 + 9:

File Cases
test_backend_step_cost.py 29 (18 plain + 3 parametrised over non-durations + 7 over the readers + 1)
test_backend_provenance.py 15 (11 plain + 4 parametrised over blank-half refusals)
test_backend_ladder.py 15
test_backend_interface.py 9 (4 plain + 5 parametrised, one per module under backends/)

The merge is clean — no conflict. atom/compass/__init__.py is now taken verbatim from f67618eb9, which landed its own copy while this was in review, so the add/add conflict the reviewer found is resolved on this branch rather than left for the landing.

gate_cpu.sh reports gpu: not required on every tree from the .compass-changed stamp: the diff touches atom/compass/ and tests/compass/ only, and no path in it matches scripts/compass/gpu_gate_triggers.txt. The GPU tier was therefore not run, and no claim is made about it.

ruff check and black --check are clean on all ten new files (the repository's wider ruff baseline is dirty; the gate is no new error, and these add none).

Effort: 261 code lines against the 200–300 estimate (227 at the first push, 256 after round 1). Measured by AST — ast.parse, strip module/class/function docstrings, ast.unparse, count non-blank lines — over the six source files: cost.py 109, ladder.py 68, provenance.py 56, base.py 22, backends/__init__.py 5, compass/__init__.py 1. The four test files add 327 by the same method and are not counted against the estimate.

No blocking issues.

🤖 Generated with Claude Code

….5, #23)

Four things a cost backend needs before there is a cost backend: the
interface itself, the step cost it returns, the vocabulary every number in
that cost is labelled with, and the order sources are consulted in.

The interface is two methods. `estimate(batch_view)` prices one step and
`describe()` says what the backend is answering from. `batch_view` is a
projection the caller prepares -- plain numbers, no engine objects -- so the
package imports nothing from the rest of ATOM and runs anywhere Python does.
A test reads the sources and asserts that, rather than trusting it.

`StepCost` does not store a total. `seconds` is folded from `terms` on every
read, so no sequence of calls leaves a whole that disagrees with its parts,
and an empty breakdown is refused outright -- the mean of an empty sample is
exactly 0.0, which reports a step that took no time rather than a model that
was missing. The fold is a left fold from 0.0 in the stored order, exposed as
`fold_seconds` so that a reader checking a breakdown against its total sums
it the same way and gets the same bits. Float addition is not associative and
the same terms in a different order are a different number; a test asserts
that with 0.1 + 0.2 + 0.3.

Every cost carries a `Provenance`, and neither it nor its species has a
default, so an unlabelled number takes a deliberate lie rather than an
oversight. Five species: analytical, measured, fitted, interpolated,
extrapolated. A `Refusal` is a value, not an exception -- a source and a
reason, both required -- and it is stamped onto the provenance of whatever
cost stood in for it.

`Resolver` walks sources in a fixed order and takes the first answer. It
cannot produce an unannotated answer from a lower source: each refusal
collected on the way down is carried on the resolution, and the first one is
stamped on the answer, so a run that drops from a measured price to a
computed one says so per term. When nothing answers, `CostRefused` carries
the whole list rather than the first entry, so one run names every gap
instead of one gap per run.

`ProvenanceMix` accumulates a run: seconds by species, and refusals by
count, by fraction of steps and by fraction of predicted seconds. The three
are reported together because a run can refuse 2% of its steps and 40% of
its time.

Tests: 43 new CPU-only cases in tests/compass/.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread atom/compass/backends/cost.py Outdated

@property
def refused_second_fraction(self) -> float:
return self.refused_seconds / self.seconds if self.seconds > 0.0 else 0.0

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Blocking — principle 7 (and principle 6), the zero-TTFT defect reproduced one layer above the one that refuses it.

StepCost refuses an empty breakdown, and this module's own docstring says why: "the mean of an empty sample, which is not an error, is not zero-ish, and is exactly 0.0 — a precise and entirely fictional answer that reads as a fast step rather than as a missing model." ProvenanceMix then returns exactly 0.0 from both fractions when steps == 0, and test_backend_step_cost.py:125 blesses it.

Measured, in this branch:

ProvenanceMix() -> steps=0  refused_step_fraction=0.0  refused_second_fraction=0.0
                   seconds_by_species()={}  reasons()={}

This is the layer the admissibility gate reads (08 D49 §3: a run with more than 5% of predicted seconds refused is not acceptance evidence). A run that produced no steps at all — a harness that never connected, a predict pass that refused at step 0, a mix that was constructed but never recorded — reports refused_second_fraction == 0.0 and passes that gate. "No refusals" and "no data" are the same number, and the number is the reassuring one.

refused_step_fraction and refused_second_fraction are the only two properties in the package that answer a question they have no basis for. Suggest the same treatment the sibling class already gets:

@property
def refused_step_fraction(self) -> float:
    if not self.steps:
        raise ValueError(
            "no steps were recorded; a refused fraction of nothing is not 0%, "
            "it is unanswered"
        )
    return self.refused_steps / self.steps

refused_second_fraction's self.seconds > 0.0 guard needs the same split: zero recorded seconds across a non-zero number of steps is a different fact from zero steps, and today both read 0.0.

Comment thread atom/compass/backends/provenance.py Outdated
which is the one an operator has to go and measure. Later declines are
kept on the resolution, not here.
"""
if self.refusal is not None:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Blocking — a silent fall-through is producible here, and it contradicts this method's own stated rule.

after() returns self unchanged when the provenance already carries a refusal. The docstring says "The first refusal wins: it names the source that should have answered." Under composition, the refusal that wins is the last one, not the first.

The shape is the one W1.6 and after will build: a rung that is itself resolver-backed, so its answer already carries an inner refusal. Measured on this branch:

inner = Provenance(Species.ANALYTICAL, "roofline").after(
    Refusal("inner_price_list", "unpriced leaf"))
Resolver([Declines("price_list", "no entry for gemm"),
          Answers("law", 0.005, prov=inner)]).resolve("attn")

  Resolution.declined -> [("price_list", "no entry for gemm")]
  term provenance     -> "refused(inner_price_list: unpriced leaf) -> analytical (roofline)"
  term.refusal.source -> "inner_price_list"      # not price_list
  ProvenanceMix.reasons() -> {("inner_price_list", "unpriced leaf"): 1}

price_list declined first, chronologically and in ladder order. It is the rung an operator has to go and measure. It reaches the artifact nowhere, and the run-level reason counts name the wrong source. Resolution.declined still holds it, but Resolution is not what becomes a StepCost (see the separate comment on ladder.py:133), so nothing downstream can recover it.

The stand-in is priced, the total is tidy, and the record says a different gap exists than the one that does. That is the failure shape the module docstring above it says it designs out.

Two ways out, both small:

  • make Provenance.refusal a tuple[Refusal, ...] in ladder order and have after() prepend rather than defer — which also fixes the middle-rung loss; or
  • keep the single slot and have after() prepend-by-replacement (outermost wins), documenting that the innermost survives on the Resolution.

The first is the one that makes the claim true rather than narrower.

Comment thread atom/compass/backends/ladder.py Outdated
answer = CostTerm(
answer.name, answer.seconds, answer.provenance.after(declined[0])
)
return Resolution(answer, source.name, tuple(declined))

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Principle 7 — every refusal is on Resolution.declined, and Resolution is where they stop.

The claim in the PR body is "one run names every gap, instead of one gap per run." That holds for an exhausted ladder (CostRefused carries the list). It does not hold for a fall-through, which is the common case.

A Resolution is not what a step is built from — StepCost takes CostTerms, a CostTerm carries one Provenance, and a Provenance carries one Refusal. So everything between the first decline and the answer is dropped at this boundary. Measured on this branch, three rungs, two declining:

Resolution.declined      -> [("price_list","no entry"), ("nearest_key","outside range")]
StepCost([res.term]).rows() -> ("attn", 0.005, "refused(price_list: no entry) -> analytical (law)")
ProvenanceMix.reasons()  -> {("price_list","no entry"): 1}

nearest_key declined on every step of the run and appears in no count. A rung that never answers is invisible to the coverage report whose job is to say what to measure next — and a nearest-neighbour rung that is silently dead is exactly the kind of thing that only shows up as an accuracy shortfall three tasks later.

Fixing Provenance.after to hold a tuple (see the provenance.py:103 comment) closes this one too: stamp all of declined, in order, and ProvenanceMix.reasons() then counts every gap a run actually hit.

Comment thread atom/compass/backends/ladder.py Outdated
)
if declined:
answer = CostTerm(
answer.name, answer.seconds, answer.provenance.after(declined[0])

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

08 D49 §1 requires the tag to name the rung that actually answered. This leaves that to the source's convention.

The design's wording is "tagged provenance=refused(<reason>) naming the rung that actually answered." The resolver knows that name — it is source.name, and it goes onto Resolution.source on the next line — but it is not stamped onto the provenance. Whether the artifact names it depends on the source having populated detail itself. Nothing requires that, and Provenance.detail defaults to "".

The PR's own test already shows the failing rendering (test_backend_provenance.py:75, Provenance(Species.ANALYTICAL) with no detail). Reproduced through the resolver:

Resolution.source -> "analytic_law"          # the resolver has it
artifact row      -> ("attention", 0.005,
                      "refused(price_list: unpriced leaf) -> analytical")

analytical is a species, not a rung. Two rungs at the same species are indistinguishable in the record, which is the counting problem Refusal(source, reason) was given a required source to avoid — the same requirement is absent on the answering half.

This is the one thing in the diff where a stated property is carried by convention rather than by shape, which is the distinction the rest of the package is built around. One line, at the stamp site:

if declined:
    stamped = answer.provenance.after(declined[0])
    if not stamped.detail:
        stamped = Provenance(stamped.species, source.name, stamped.refusal)
    answer = CostTerm(answer.name, answer.seconds, stamped)

A required source field on Provenance would be stronger still and would make seconds_by_species() groupable by rung as well as by species — worth considering before W1.6 builds against this.

Comment thread atom/compass/backends/cost.py Outdated

def rows(self) -> tuple[tuple[str, float, str], ...]:
"""The breakdown as an artifact writes it: name, seconds, provenance."""
return tuple((t.name, t.seconds, str(t.provenance)) for t in self.terms)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Non-blocking, principle 7 at the artifact boundary — rows() re-folds but does not re-decompose.

The docstring calls this "the breakdown as an artifact writes it", and the fold property is real: fold_seconds(s for _, s, _ in rows()) == step.seconds, bit for bit. But the third column is one rendered string, so everything except the name and the seconds is lost to prose:

("mlp",       0.006, "measured (step-level)")
("attention", 0.005, "refused(price_list: unpriced leaf) -> analytical")

From that, a reader with only the artifact cannot recompute seconds_by_species() and cannot recover (source, reason) without parsing refused(...) -> .... 08 D49 §2 asks the run artifact to carry the refused count, the two fractions and the distinct reasons with counts; today those exist only on the live ProvenanceMix, and the writer is (correctly) out of scope for this PR.

Suggest widening the row before a writer is built against it — (name, seconds, species, detail, refusal_source, refusal_reason) or a small NamedTuple — so the artifact is re-decomposable and not only re-summable. Flagging it here because rows() is the shape W1.6 and the artifact task will both read, and it is cheaper to widen now than after two consumers exist.

Comment thread atom/compass/backends/cost.py Outdated

def record(self, step: StepCost) -> None:
self.steps += 1
self.seconds += step.seconds

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Non-blocking, two claims that do not survive checking — the run-level total is stored, and fold_seconds is not the only summation.

1. The rule StepCost enforces structurally is enforced nowhere here. steps, seconds, refused_steps and refused_seconds are plain public attributes with no invariant tying them to _species_seconds. Measured:

mix.record(StepCost([CostTerm("a", 0.1, M), CostTerm("b", 0.2, M)]))
mix.seconds = 0.0
  mix.seconds              -> 0.0
  mix.refused_second_fraction -> 0.0
  mix.seconds_by_species() -> {MEASURED: 0.30000000000000004}

The whole disagrees with the parts and nothing notices — the exact state StepCost exists to make unrepresentable, at the layer a run reports from. Making seconds a property folded from _species_seconds, and the four counters private, would carry the same rule one level up for about four lines.

2. "The only summation this package performs" (line 37) is not true. Five additions happen outside fold_seconds: cost.py:137-139 (seconds_by_species), :166, :169, :171-173. All five are left folds in stored order, so the bit-for-bit property survives — but by coincidence, not by construction, and the docstring is what a later editor will read before adding the sixth. Either route them through fold_seconds or narrow the claim to what is actually guaranteed (the fold of a breakdown).

For the record, the fold claim itself reproduces exactly:

forward 0.1+0.2+0.3 -> 0.6000000000000001
reverse 0.3+0.2+0.1 -> 0.6
mix.seconds == fold_seconds(mix.seconds_by_species().values())  -> True

Comment thread atom/compass/backends/cost.py Outdated
@property
def seconds(self) -> float:
"""The total, folded from the terms in their stored order."""
return fold_seconds(term.seconds for term in self.terms)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Non-blocking — the guard holds at construction, not on read, and the class is not sealed.

I tried to build an aggregate without its decomposition by every route I could find. Results on d892626af:

Route Outcome
StepCost([]) refused
dataclasses.replace(step, terms=()) refused — the custom __init__ takes terms by keyword, so replace revalidates
copy.deepcopy preserved, valid
pickle round-trip of a valid step valid
object.__setattr__(step, "terms", ()) seconds == 0.0
step.__dict__["terms"] = () seconds == 0.0
StepCost.__new__ + setattr seconds == 0.0
pickle of an already-hacked object seconds == 0.0
subclass, valid terms, seconds overridden seconds == 99.0, fold(rows()) == 0.3

dataclasses.replace holding is genuinely good and is the route I expected to break. The object.__setattr__ family I accept as the deliberate lie the test docstring names — Python cannot stop it and pretending otherwise is not worth code.

The last row is the one I would close. It needs no bypass: a plain subclass with valid terms and an overridden seconds makes the whole disagree with the parts through the public API, and a cached total on a subclass is exactly what someone adds when profiling shows the fold in a hot loop. Both that and the setattr family are closed by re-checking the invariant where it is read rather than only where it is written:

@property
def seconds(self) -> float:
    if not self.terms:
        raise ValueError("a step cost with no terms has no total to report")
    return fold_seconds(term.seconds for term in self.terms)

Three lines, and the property becomes true on every read instead of once. Consider @typing.final on StepCost as well, or a __init_subclass__ that refuses an override of seconds — W1.6 will be the first thing tempted to subclass this.


def test_an_empty_run_reports_zero_rather_than_dividing_by_zero():
mix = ProvenanceMix()
assert mix.refused_step_fraction == 0.0

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This test is the one place the PR argues for the defect it is otherwise about.

0.0 here is not "rather than dividing by zero" — it is the same answer a run with a thousand clean steps gives, and the caller cannot tell them apart. The docstring one file over says why that matters: "the mean of an empty sample, which is not an error, is not zero-ish, and is exactly 0.0."

See the comment on cost.py:184. When that is fixed, this test should assert the refusal instead, and the name reads better as something like test_a_run_with_no_steps_refuses_rather_than_reporting_zero_percent.

@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Review record — W1.5 / #23, round 1

Agent-authored review. Read design/README.md's eight principles and 02 D10–D12 before the diff, per AI_DEV_RULES.md.

Verdict: CHANGES REQUESTED. Two findings block, both one-line-to-five-line fixes, both in the exact failure class this PR was written to close:

  1. ProvenanceMix reports 0.0 for both refused fractions on an empty run (cost.py:180,184, blessed by test_backend_step_cost.py:125) — the zero-TTFT defect reproduced one layer above the constructor that refuses it, at the layer 08 D49 §3's 5%-of-seconds admissibility gate reads. Principle 7, and principle 6.
  2. A silent fall-through is producible (provenance.py:103 with ladder.py:129-132) — Provenance.after returns self when the provenance already carries a refusal, so in a composed ladder the resolver's own refusal is dropped from the term and ProvenanceMix.reasons() names the wrong rung. It contradicts the method's own "the first refusal wins". Principle 6.

Two more should land with them, since they are in the same two functions: middle-rung refusals never reach the artifact or the run mix (ladder.py:133), and the artifact can fail to name the rung that answered, which 08 D49 §1 requires (ladder.py:131). Three non-blocking findings and the judgement calls are below.

The design conformance is otherwise good and the four things I was asked to test rather than read mostly hold. This is a request for four small changes to an interface that is close, not a rejection.


What I checked, and what reproduced

Everything below was run, not read. Gates on node 18, xiaobizh_n18_cpu, git archive + docker cp into /tmp/w15gates/{branch,control} (never the shared mount), tarball md5 verified host → node → container, PYTHONPATH asserted by import atom; print(atom.__file__) before reading any count.

Claim Result
Control c19710bcd 4030 passed, 149 skipped, 3 xfailed — reproduces
Branch d892626af 4073 passed, 149 skipped, 3 xfailed, GATE_CPU_RC=0 — reproduces
4030 + 43 = 4073 arithmetic correct; delta is exactly the four new files (43 collected, rc=0, run standalone)
11 + 14 + 9 + 9 correct per file
GPU tier not required confirmed — GATE_CPU_RC=0 includes the blind-spot check; no path matches gpu_gate_triggers.txt
227 AST code lines reproduces exactly, per file: cost.py 92, ladder.py 65, provenance.py 42, base.py 22, backends/__init__.py 5, compass/__init__.py 1. Independent re-measure, same method. Within 200–300.
208 test lines reproduces (44 / 70 / 29 / 65)
ruff check, black --check both clean on all ten files. (ruff format --check would reformat test_backend_interface.py:88-92; ruff format is not this tree's gate, so this is a note, not a finding.)
No design-doc references in code or emitted data confirmed. No D*, T*, P0.*, W*.*, no "principle N", no backticked doc numbers, in source, tests, docstrings or any emitted string. Tier's "0"/"a"/"b" and ANALYTIC/COARSE/OP_LEVEL are the README glossary's own domain vocabulary for the cost-tier axis, not a citation.

Fold order (claim 2). Reproduces: forward 0.6000000000000001, reverse 0.6, rows() re-folds to seconds bit for bit. But fold_seconds is not the only summation in the package — five additions live outside it (cost.py:137-139,166,169,171-173). All five happen to be left folds in stored order, so the property survives by coincidence rather than by construction. Detail on cost.py:166.

Provenance vocabulary (claim 3). Checked against the design rather than taken on trust. README.md:462 gives exactly analytical / measured / fitted / interpolated / extrapolated, flat, and :468 lists refusal as its own glossary term rather than a sixth species — so both the five names and the decision not to add a sixth are the design's, not invented. 02 D11's empirical/measured spelling is the same vocabulary namespaced; the glossary is the canonical form and the PR follows it. Both carried rules check out: no PRICED exists and the unit goes in detail; nothing anywhere makes a file read-back a distinct species. Provenance() and Provenance("measured") both raise; CostTerm(name, seconds) raises. I could not build an unlabelled cost by any route.

Principle 7, structural or customary (claim 1). Nine construction routes tried, including dataclasses.replace, __dict__ mutation, __new__, subclassing and pickling. dataclasses.replace(step, terms=()) refuses — the custom __init__ takes terms by keyword so replace revalidates, which is the route I most expected to break and it holds. The object.__setattr__ / __dict__ / __new__ family reaches seconds == 0.0; I accept those as the deliberate lie the test docstring names. A plain subclass with valid terms and an overridden seconds needs no bypass at all and makes the whole disagree with the parts through the public API. Full table and a three-line fix on cost.py:114.

Refusal counting. All three readings are computable and, for a single-rung refusal, correct: refused_steps, refused_step_fraction, refused_second_fraction, plus reasons() keyed (source, reason) and seconds_by_species() — which is 08 D49 §2's list in full. Two caveats the gate task needs: reasons() under-counts whenever a ladder falls through more than one rung (ladder.py:133), and the empty-run hole above. One thing to state rather than fix: refused_steps is any term refused while refused_seconds is only refused terms, so the two fractions are at different granularities by design. That is the right choice, but 08 D49 §3's 5% threshold does not say which, and a run can be 100% refused by steps and 4% by seconds. W1.6 or the validation task should record which one the threshold is against.


Rulings on the recorded judgement calls

1. batch_view untyped — right call, keep it. 02 D11 fixes only that it is a projection, not the object; it fixes no fields. 02 D12 then shows why guessing would be wrong: the features M1 needs include detailed_sqsq / detailed_sqsk / detailed_sk, which are currently gated behind profile_active and ATOM_ENABLE_DETAILED_ANNOTATION, and the CUDA-graph rung, whose wrong derivation already cost −22.6% at rung 16 once. Naming a type here would fix a shape nobody has measured, against principle 3. The enforcement that actually matters is tested rather than asserted, which is the correct trade.

W1.6 does not need a type here; it needs one in its own module. Two things to carry forward: (a) define the projection as a frozen dataclass W1.6 owns, and leave CostBackend.estimate at Any, so the interface does not learn the runner's shape; (b) extend test_the_package_imports_nothing_from_the_engine to whatever module holds that dataclass, or the no-engine-imports property stops at this package's boundary while the projection type sits just outside it.

2. refused is not a sixth species, rendered refused(source: reason) -> species (detail) — accept the shape, fix the rendering. Right axis: species answers how was this obtained, refusal answers did the first choice answer, and folding them would make seconds_by_species() unable to say what the stand-in actually was. The README glossary agrees. The rendering reads left to right and greps cleanly. The change it needs is that the right-hand side must name the rung structurally, not by the source's convention — 08 D49 §1 asks for that by name and today refused(price_list: unpriced leaf) -> analytical is reachable. See ladder.py:131.

3. Every decline is a refusal — accept. The alternative needs a rule for which declines are quiet and nothing in the design supplies one; principle 6 makes loud the right default. Note that the choice currently buys less than it should, because the loud declines are discarded at the Resolution → StepCost boundary (ladder.py:133).

4. Flat terms, hierarchy in the name — accept. Principle 3, and it matches how the consumers actually read a breakdown: seconds_by_species() and reasons() are groupers, not tree walkers. 04's opaque leaves are priced and not decomposed, so op-level rows will be flat anyway; the pressure will come from per-layer rows, and attention.decode handles that without a tree. Revisit only if a consumer appears that genuinely walks.

5. CostRefused raised, not returned — accept. 02 D11 says a backend with no samples must raise. A Refusal value everywhere there is a cost to attach it to, and an exception only where there is none, keeps that true without a sentinel.

6. The parametrised interface test moving the CPU total — not a problem for the gate. The gate is a delta between a control and a head at the same base, so a count that is a function of the tree is fine; it is only a problem for a wave-level claim. W1.6's PR body should state the +1 per module it adds under backends/, or its own delta will read as a stray test. Keeping the guard automatic is worth more than a constant count.


Accepted with reservation

  • rows() re-folds but does not re-decompose (cost.py:144) — species and (source, reason) are flattened into one rendered string, so 08 D49 §2's artifact requirements cannot be met from the rows alone. Out of scope here (no writer in this diff), but the row shape should widen before two consumers exist.
  • ProvenanceMix's counters are public and mutable (cost.py:156-176) — the run-level total is stored, not folded, and can be set independently of _species_seconds. Four lines make it the same shape as StepCost.
  • ruff format disagreement on one test file. Not this tree's gate; noted only so it is not a surprise later.

What W1.6 should watch

  1. A rung can fall back internally and the ladder cannot see it. Resolver guarantees no unannotated answer from a lower rung, which is true of the class and cannot be true of what is inside a rung. A CostSource that quietly computes a roofline instead of calling self.refuse yields fell_through == False, refused_steps == 0, reasons() == {} — the only trace left is Species.ANALYTICAL in the mix. Verified on this branch. The species axis is therefore load-bearing as the last line of defence, and every M1 source must be honest about its own species even when it answers.
  2. Do not cache a total on a StepCost subclass. See cost.py:114.
  3. The projection type and its import guard, per ruling 1.
  4. State which granularity 08 D49 §3's 5% is against before anything reads refused_second_fraction as a gate.

Not checked

  • The GPU tier. GATE_CPU_RC=0 says it is not required for this diff; I did not run it and make no claim about it.
  • Whether Tier's three values are the right partition for anything beyond this interface — nothing in the diff consumes them, and Tier is declared and not checked, which the PR body already records as left undone.
  • Concurrency. ProvenanceMix is not thread-safe and nothing here says it must be; 02 D11 does not settle whether a run accumulates from one thread.

Re-review on push. Findings 1 and 2 are what the verdict turns on.

…#23)

Review found the empty-sample defect reintroduced one layer above the
constructor that refuses it, and a route by which a fall-through goes
unrecorded. Both are small, and two related gaps land with them.

`ProvenanceMix` returned 0.0 for both refused fractions on a run with nothing
in it, so "refused nothing" and "recorded nothing" were the same number at
exactly the layer that reads it to decide whether a run is evidence -- and of
the two readings the wrong one is the reassuring one. Both fractions now
refuse an empty denominator and say which one is missing; `is_empty` is there
for a caller that wants to ask first. A run whose steps are all free keeps its
step fraction, which is answerable, and refuses only the seconds fraction.

`Provenance.after` kept the existing refusal and discarded the incoming one,
which is right for a flat ladder and wrong for a rung that is itself
resolver-backed -- the outer refusal happened first, so the record named the
inner rung and the reason counts named the wrong source. It is now
`resolved(source, declined)`, which prepends rather than replaces: outer
refusals go in front, the earliest refusal is genuinely the first, and nothing
is dropped.

Two more from the same two functions. A provenance now carries every refusal
collected on the way down, not just the one that renders, so a middle rung's
reason reaches the artifact and the reason counts instead of vanishing on any
multi-rung fall-through. And the resolver stamps the name of the rung that
answered on every answer, not only after a fall-through, so a row can no
longer render as `-> analytical` with nothing saying which rung produced it.
A composed answer reads `layered/analytic_law`.

The decomposition claim is now true on every read rather than once at
construction: the breakdown is re-checked inside `seconds` and `rows`, so an
object edited past its constructor fails when used, and `__init_subclass__`
refuses a subclass that shadows `seconds`, `terms` or `rows` -- overriding a
property needs no bypass at all and was the one route to a plausible number
with nothing to do with its breakdown. Seven routes tried, seven refused.

Every addition of seconds now goes through `fold_seconds`, via `fold_step` for
running totals. It previously held by coincidence in five places, which is a
weaker property than the one claimed for it; the only `+` left outside those
two functions counts steps and reasons, which are integers.

Tests: 54 cases, up from 43.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Both blocking findings and all three secondary ones are fixed in 1e03d3979, pushed on top of d892626af — no force-push. The PR body is updated, including the What review changed table and the corrected fold claim.

Blocking 1 — empty run. refused_step_fraction and refused_second_fraction now refuse an empty denominator instead of returning 0.0, and they refuse separately: a run whose steps are all free keeps its step fraction, which is answerable, and refuses only the seconds fraction. is_empty is there for a caller that wants to ask first. test_backend_step_cost.py:...::test_a_run_with_no_steps_refuses_its_fractions replaces the test that blessed the zero.

Blocking 2 — dropped chain. Provenance.after is gone. resolved(source, declined) prepends rather than keeping-or-replacing, so the outer refusal — which happened first — is first, and nothing is discarded. test_a_resolver_backed_rung_keeps_both_halves_of_the_chain asserts your exact reproduction: refusals ["price_list", "inner_price_list"], renderer names price_list, answering name layered/analytic_law.

Middle rungs / answering name. A provenance now carries the whole refusal chain, and the resolver stamps the answering rung on every answer rather than only after a fall-through. test_every_source_passed_over_reaches_the_answer_and_the_count runs a three-rung fall-through through ProvenanceMix and asserts both reasons are counted; test_the_artifact_names_the_rung_that_answered asserts the rendered row.

Subclass drift. __init_subclass__ refuses a subclass that redefines seconds, terms or rows, and _checked now runs inside seconds and rows, so the claim holds on every read rather than once at construction. I re-ran your routes: all seven refuse — replace, object.__setattr__, __dict__, __new__, pickle-of-a-hacked-object, the subclass (at class definition), and rows() on a hacked object. copy and pickle of a valid step round-trip and fold identically.

I did both the property guard and __init_subclass__ rather than the guard alone: a guard inside seconds cannot see a subclass that replaces seconds, so it closes the five object-editing routes and the subclass route needs the definition-time refusal. Flagging in case you meant one mechanism for all six.

Fold claim. Corrected, and made true by construction rather than restated: fold_step(total, value) is the one-step form of fold_seconds, the five running totals go through it, and seconds_by_species groups first and folds each group. The only + left outside those two functions counts steps and reasons, which are integers. test_a_running_total_is_the_same_fold_as_a_batched_one pins it.

W1.6 note taken: recorded in the PR body under the interface, so it is in the handoff path rather than only in this thread.

Gates re-run, node 18, xiaobizh_n18_cpu, git archive snapshots md5-verified host → node → container:

Control c19710bcd Branch 1e03d3979
Result 4030 passed, 149 skipped, 3 xfailed 4084 passed, 149 skipped, 3 xfailed
GATE_CPU_RC 0 0
GPU tier not required not required

4030 + 54 = 4084, decomposing 15 + 19 + 11 + 9. Both runs read 149 skipped, so the one-in-four 4029/150 flake did not appear in either and no re-run was needed. ruff and black clean on all ten files. AST lines 256 (from 227), still inside the 200–300 estimate: cost.py 108, ladder.py 64, provenance.py 56, base.py 22, inits 5 + 1.

Still draft, not merged, #23 still open.

Comment thread atom/compass/backends/ladder.py Outdated
"""
declined: list[Refusal] = []
for source in self._sources:
answer = source.price(request)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Blocking — the chain still drops on the refusal path, and a ladder aborts where it could have answered.

Round 1's blocker was the composed answer path, and it is genuinely closed — I took it to four levels below and every refusal arrives, in chronological order, with the composed name. The same composition on the refusal path is not closed, and this line is where it escapes: source.price(request) is called with no handling for the one exception this module itself defines.

A resolver-backed rung is the shape the docstring above names ("A rung may itself be resolver-backed"), and it is the shape the PR's own test helper implements (test_backend_ladder.py::Layered, return self._inner.resolve(request).term). Run that helper against an inner ladder that exhausts, measured on 1e03d3979:

dead  = Resolver([Declines("inner_price_list", "no entry"),
                  Declines("nearest_key", "outside range")])
outer = Resolver([Declines("price_list", "top rung unpriced"),
                  Layered("layered", dead),
                  Answers("roofline", 0.009)])
outer.resolve("attention")

  CostRefused.declined -> ["inner_price_list", "nearest_key"]

Two things happen, both of them the failure this class exists to prevent:

  1. price_list is gone. The inner CostRefused propagates straight out of this loop, so the outer declined list — which holds the rung an operator would go and fix first — is discarded and never reaches the raised exception. The module docstring says "CostRefused is raised carrying every refusal in order, so the caller gets the complete list of what would have to be measured rather than the first item of it." Under composition it carries the inner list and none of the outer one. That is the identical shape to round 1's blocker, one path over.
  2. roofline is never consulted. It can answer. The ladder refuses instead. A rung that exhausts internally kills the whole ladder rather than being one more decline — so composition does not just lose the record, it changes the result.

There is no correct way to write a resolver-backed rung against today's interface. The other implementation — catching and re-refusing, which is what price() -> CostTerm | Refusal strictly asks for — behaves right and loses the decomposition instead, because refuse() takes one string and Refusal has no way to carry a sub-chain:

except CostRefused as e:
    return self.refuse("; ".join(f"{r.source}: {r.reason}" for r in e.declined))

  ProvenanceMix.reasons() -> {
    ("price_list", "top rung unpriced"): 1,
    ("layered", "inner_price_list: no entry; nearest_key: outside range"): 1,
  }

inner_price_list and nearest_key are no longer (source, reason) pairs and cannot be counted, grouped or acted on — which is exactly what the middle-rung fix bought on the answer path, given back on the refusal path. For contrast, the same ladder where the inner rung answers is correct today: {("price_list", ...): 1, ("inner_price_list", ...): 1}.

The fix is in this loop and is about five lines — treat the module's own exception as what it is, a decline:

try:
    answer = source.price(request)
except CostRefused as inner:
    declined.extend(inner.declined)
    continue

That closes both halves at once: the outer refusals survive into the raised CostRefused, the inner ones arrive as their own (source, reason) pairs, and a lower rung still gets asked. It also makes the two composition paths symmetric, which is the property the round-1 fix established for answers.

Comment thread atom/compass/backends/cost.py Outdated
three, so the class is refused at definition rather than at use.
"""
super().__init_subclass__(**kwargs)
shadowed = sorted({"seconds", "terms", "rows"} & set(cls.__dict__))

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Secondary — right mechanism, short coverage. The seal names the total but not the refusal decomposition, and the refusal decomposition is what this PR is for.

The seven routes reproduce: all seven refuse on 1e03d3979, including dataclasses.replace, the three object-editing routes, pickle-of-a-hacked-object, rows() on one, and the subclass at class-definition time. And the ruling you asked for is below.

The gap is the set on this line. {"seconds", "terms", "rows"} is the total and its rendering; ProvenanceMix.record reads four more members off a StepCost, and none of them is guarded. Measured:

class Quiet(StepCost):
    @property
    def refusals(self): return ()
    @property
    def is_refused(self): return False

Quiet([CostTerm("attn", 0.1, ANALYTIC.resolved("law", [Refusal("price_list", "x")]))])
  .is_refused -> False        # class definition accepted, no bypass used

Recorded into a mix, that step is not counted in refused_steps, contributes nothing to refused_seconds, and adds nothing to reasons(). A run of them reports refused_step_fraction == 0.0 — a real number, over real steps, that says a fall-through never happened. That is the silent fall-through at the run layer, reached by subclassing and nothing else. refused_seconds and seconds_by_species go the same way:

class D(StepCost):  @property refused_seconds -> 99.0        accepted
class D(StepCost):  def seconds_by_species -> {MEASURED: 99.0}  accepted

Suggest either widening the set to every member a consumer reads —
{"seconds", "terms", "rows", "refusals", "is_refused", "refused_seconds", "seconds_by_species"}
— or, simpler and with nothing to keep in sync, refusing any subclass that defines any of StepCost's public names. Nothing in the design wants a StepCost subclass; the narrower set is a list that has to be maintained every time a reader is added, and this is the first time it has been added to.


Your push-back, ruled: you are right, and my round-1 sentence was wrong.

A guard inside seconds cannot close the subclass route. The two routes are disjoint by construction:

  • instance-state corruption (object.__setattr__, __dict__, __new__, pickle-of-hacked, replace) — the parent's reader still runs, so a check placed inside it observes the corruption;
  • class-level replacement of the reader — the parent's reader never runs, so no code inside it can observe anything. There is nothing a property body can do about a property that is not it.

typing.final is not a third option: it is static-only and has no runtime effect. So the definition-time refusal is separately necessary and two mechanisms is the correct answer. My round-1 comment's claim that "both that and the setattr family are closed by re-checking the invariant where it is read" was simply false; its next sentence already pointed at __init_subclass__, which is what you built.

One single mechanism does exist, for completeness: drop seconds from the class entirely and make the total a module-level total(step) that every consumer calls. With no attribute to shadow, the subclass route disappears and the one check lives in the one function. I would not take it here — it moves the invariant off the type that owns it and turns every read site into a convention to remember, which is the thing this package is built to avoid. Two mechanisms, with the set above widened, is the right shape.

def fold_seconds(values: Iterable[float]) -> float:
"""Sum in the given order, left fold from 0.0.

The only summation this package performs, so that a total and a check of

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Non-blocking — the round-1 finding is fixed; the docstring's claim is now one step wider than what it buys.

The correction holds and it holds by construction, which is what I asked for. Enumerated every Add node in the package by AST on 1e03d3979 — exactly five, and no sum() anywhere:

cost.py:49       total += value                    inside fold_seconds
cost.py:205      self.steps += 1                   int
cost.py:208      self.refused_steps += 1           int
cost.py:216      self._reasons.get(key, 0) + 1     int
provenance.py:130  tuple(declined) + self.refusals  tuple concat, not arithmetic

Every addition of seconds now goes through fold_seconds or fold_step, and the three integer counters really are integers on every path inside the class: steps and refused_steps are initialised to 0 and only ever += 1, and _reasons values are only ever written by that one get(key, 0) + 1. (They are public attributes, so an outside assignment can still make them floats — the round-1 reservation, still open, still not blocking.)

fold_step(total, value) == fold_seconds((total, value)) is exact rather than approximately so, because 0.0 + t is bit-identical to t for every reachable t: CostTerm refuses negatives and non-finites, so a running total is always non-negative and finite. Running-versus-batched reproduces on adversarial sequences too — [1e16, 1.0, -0.0, 1e-17] and [1e308, 1e-300] both agree bit for bit.

What the claim does not cover. Line 44's "The only summation this package performs, so that a total and a check of that total agree bit for bit" is true of re-summing in the same order (rows() → seconds reproduces exactly, verified) but not of re-summing a grouping, because grouping re-associates. seconds_by_species() folds within each species and the caller folds across species, which is a different parenthesisation of the same terms:

terms: fitted 0.2, measured 0.7, measured 0.2, fitted 3.3, fitted 0.1
  step.seconds                        -> 4.499999999999999
  fold_seconds(by_species.values())   -> 4.5

Random search: 427 of 4000 trials over plausible durations disagree in the last bit. Round 1 happened to check a case where it agreed, which is the same coincidence the original finding was about, one level down.

Nothing in the diff claims the mixture re-sums to the total, so this is not a false claim — but 08's artifact carries both numbers, and the first writer that prints a mixture beside a total will print one that does not add up about a tenth of the time. Worth one sentence on fold_seconds or seconds_by_species saying that grouping is a re-association and only the row order round-trips exactly, so the artifact task does not have to rediscover it.

raise ValueError(f"{self.name}: {self.seconds} is not a finite duration")
if self.seconds < 0.0:
raise ValueError(f"{self.name}: {self.seconds} s is negative")
if self.provenance.is_refused and self.seconds <= 0.0:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Non-blocking — this check and "a zero term is fine when nothing refused" collide inside the resolver, and the collision surfaces as a raw ValueError.

The rule is right and the reason for it is right. What it cannot currently express is a rung that legitimately answers zero after a higher rung declined — and since Resolver now stamps every refusal onto every answer, that is any zero answer anywhere below the top of a ladder. Measured on 1e03d3979:

Resolver([Declines("price_list", "no entry"), Answers("free_law", 0.0)]).resolve("collective")
  ValueError: collective: a refusal was priced at 0.0 s. ...

Resolver([Answers("free_law", 0.0)]).resolve("collective")
  -> 0.0        # same source, same answer, no fall-through

"No collective on this step" is the example the PR body itself gives for a legal zero, and it is exactly the kind of term a lower rung would produce. Today its legality depends on whether an unrelated rung above it happened to decline, and when it is illegal the ladder neither answers nor refuses — it raises ValueError out of Resolver.resolve, which is not in resolve's documented outcomes and is not something a caller can count as a refusal.

Not blocking: no number is invented and nothing is silent. But the two rules are stated independently and interact, so it should be a decision rather than a discovery. Either the zero-priced-refusal check applies only when the answering term is the stand-in for the refusal (which is what the message describes), or a zero answer below a declined rung is genuinely illegal — in which case Resolver should convert it into a Refusal naming the answering rung rather than letting a constructor exception out.

@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Review record — W1.5 / #23, round 2

Agent-authored review. Read design/README.md's eight principles and 02 D10–D12 before the diff, per AI_DEV_RULES.md. Range d892626af..1e03d3979; 1e03d3979 is the head as of posting.

No round-1 finding survived. All five — the two blocking and the three secondary — are fixed, and each was re-run rather than re-read. The loop stop does not apply and need human is not applied.

Verdict: CHANGES REQUESTED, on one new finding. The composition fix is real and I could not break it on the path it was written for; the same composition on the path beside it is still open.

The chain drops on the refusal path. A resolver-backed rung whose inner ladder exhausts raises CostRefused straight out of Resolver.resolve: the outer ladder's own refusals are discarded, and a lower rung that could have answered is never consulted. It is round 1's blocker one path over, and it is reachable with the PR's own Layered test helper. Detail and a five-line fix inline on ladder.py:123.

This is cycle 2 and the fix is one try/except in one loop. Three secondary findings below, none of which need their own cycle.


Round 1, re-run

Everything below was executed. Gates on node 18 in xiaobizh_n18_cpu, git archive snapshots (never rsync), md5 verified host → node → container at every hop, PYTHONPATH asserted by the gate's own import atom check before any count was read.

Round-1 finding Status How I checked
Blocking 1 — both fractions returned 0.0 on an empty run Fixed Five-state table below; the separation is correct and I could not find an asymmetric state
Blocking 2 — Provenance.after dropped the outer refusal Fixed Composed to four levels; every refusal arrives, chronologically ordered, with the composed name
Middle-rung refusals never reached the artifact or the mix Fixed Three-rung and five-rung fall-throughs; reasons() counts every rung as its own (source, reason)
The artifact could fail to name the rung that answered Fixed resolved() refuses a blank source, so -> analytical with nothing after it is unreachable; a blank-named CostSource is refused at resolve time
Subclass drift Fixed for seconds; coverage short — see cost.py:137 All seven routes refuse; four other readers are unguarded
Fold claim Fixed, and now structural AST-enumerated every Add in the package: five, one in the fold, three integer, one tuple concat

The empty-run separation is right. I looked for a state where one fraction is answerable and the code refuses it, or the reverse, and there is none:

Mix state refused_step_fraction refused_second_fraction
nothing recorded refuses refuses
one step, all terms free 0.0 refuses, and names over 1 step(s)
one step, priced 0.0 0.0

The asymmetry that would have made this wrong — a free run that is nonetheless refused — cannot exist, and it cannot exist structurally rather than by luck: CostTerm refuses a refused term priced at 0.0, so seconds == 0.0 implies refused_seconds == 0.0 and the seconds fraction is genuinely 0/0. The step fraction over the same run is 0/N, which is a fact. Right split, right message, and the message names the denominator that is missing.

A silent fall-through is not producible on the answer path, at any depth I could build. Four levels, two declines at the bottom:

Resolver([e1, Layered(Resolver([e2, Layered(Resolver([e3, Layered(
    Resolver([e4, f4, a4]))]))]))]).resolve("attn")

  refusals -> [e1, e2, e3, e4, f4]        chronological, none lost
  source   -> "lay2/lay3/lay4/a4"
  reasons() -> 5 distinct (source, reason) pairs

Provenance.refusal is e1, the outermost and earliest, which is the rung an operator has to close. The author's framing — "first refusal wins" was right for a flat ladder and exactly backwards once a rung can itself be a ladder — is correct and is what the code now does.

The fold guarantee is structural. Exactly five additions in the package: total += value inside fold_seconds, three integer counters, and one tuple concatenation. Every addition of seconds goes through fold_seconds/fold_step, and the running-versus-batched identity is exact rather than empirical, because CostTerm refuses negative and non-finite durations, so 0.0 + total is bit-identical to total for every reachable total. Reproduces on [1e16, 1.0, -0.0, 1e-17] and [1e308, 1e-300] as well as the sharp case. One residual on cost.py:44: grouping re-associates, so a species mixture need not sum to the total — 427 of 4000 random trials disagree in the last bit. No claim in the diff is false; it is a sentence the artifact task should not have to rediscover.


The one-mechanism push-back: you are right, and I was wrong

Ruled in full on cost.py:137. Short form: a guard inside seconds cannot see a subclass that replaces seconds, because the parent's reader never runs — the two routes are disjoint by construction, instance-state corruption on one side and class-level replacement of the reader on the other, and no code inside a property body can observe a property that is not it. typing.final is static-only and is not a third option. My round-1 sentence claiming the guard closed both was false; the same comment's next line already pointed at __init_subclass__, which is what you built. Two mechanisms is correct.

For completeness, one single mechanism does exist — drop seconds from the class and make the total a module-level total(step), leaving nothing to shadow — and I would not take it: it moves the invariant off the type that owns it and turns every read site into a convention.


Gates: measured, and the control has moved

The integration branch is now f67618eb9 (CA-1, #52). This PR gated against c19710bcd, which is still its merge-base. I ran four trees rather than two, because the tree that lands is neither of the two in the PR body.

Tree Result GATE_CPU_RC
c19710bcd (merge-base, the PR's control) 4030 passed, 149 skipped, 3 xfailed 0
1e03d3979 (this branch) 4084 passed, 149 skipped, 3 xfailed 0
f67618eb9 (current integration head) 4084 passed, 149 skipped, 3 xfailed 0
f67618eb9 + this branch, conflict resolved 4138 passed, 149 skipped, 3 xfailed 0

All four read 149 skipped, so the ~1-in-4 flake appeared in none of them. gpu: not required on all four from the .compass-changed stamp.

Is c19710bcd still the right control? For the delta claim, yes — it is the merge-base, so 4084 - 4030 = 54 is attributable to this PR and to nothing else. For the landing claim, no, and the PR body should add the last row: 4084 + 54 = 4138 on the merged tree, green. Two reasons it matters. First, nobody had run that tree; I have, and it passes. Second, 4084 is now ambiguous — it is simultaneously this branch's total and today's control's total, because CA-1 also added 54 tests. A reader checking this PR against the current integration head sees the same number twice and concludes the branch adds nothing.

The merge conflict is the trivial one the PR body predicted (decision 7): atom/compass/__init__.py, add/add, both a docstring and nothing else. CA-1 landed its version. Resolving it by taking the landed file is correct — there is nothing in this branch's copy to lose — and that is what the merged tree above was built from. Note it drops the production line count from 256 to 255.

Arithmetic, re-measured independently. 4030 + 54 = 4084 correct. The decomposition 15 + 19 + 11 + 9 = 54 reproduces per file, collected standalone. 256 AST production lines reproduces exactly, per file — cost.py 108, ladder.py 64, provenance.py 56, base.py 22, backends/__init__.py 5, compass/__init__.py 1 — by my own implementation of the stated method (ast.parse, strip module/class/function docstrings, ast.unparse, count non-blank), not by re-running the author's. Inside the 200–300 estimate, +29 over round 1, which is what the five fixes cost. Tests 278, also exact (46 / 95 / 93 / 44). ruff check and black --check clean on all ten files.

No design-doc references in code or emitted data — confirmed again on the new commit. Grepped all ten files for D<n>, T<n>, P0.<n>, W<n>.<n>, M<n>, "principle N", "Gate N", and NN_name.md forms, across source, docstrings and every emitted string. Nothing.


The W1.6 note travels, but it is satisfiable vacuously

It is in the PR body under the interface section, which is the right place. As written it says: "when the batch_view type lands in this package, extend that test to cover its module too."

The condition is the one case where nothing needs doing. test_the_package_imports_nothing_from_the_engine is parametrised over PACKAGE.glob("*.py"), so a type landing in atom/compass/backends/ is already covered automatically. The case round 1 named — W1.6 defines the projection as a frozen dataclass it owns, in its own module, leaving CostBackend.estimate at Any — is the case the note does not fire on, and is the case where the guard stops short of the projection. Suggest rewording to name the actual obligation: wherever the batch_view type lands, add its module to the no-engine-imports test. Worth noting the glob is also non-recursive, so a subpackage under backends/ is not covered either.


Accepted with reservation

  • ProvenanceMix's counters are still public and mutable — carried from round 1, still not blocking. It now interacts with the fix: mix.seconds = 0.0 makes refused_second_fraction refuse rather than lie, which is the safe direction, but mix.steps = 0 leaves refused_second_fraction answering over a run the step fraction refuses to describe. It is also the only way the three integer counters become floats. Four lines would fold them the way StepCost folds.
  • A zero-second answer below a declined rung raises ValueError out of Resolver.resolve — cost.py:82. Two independently-correct rules that collide under composition, and the collision escapes as neither an answer nor a refusal.
  • object.__setattr__(step, "terms", <shorter but valid tuple>) is undetectable — but the whole still agrees with the parts, so principle 7 holds and there is nothing to close.

What W1.6 should watch

  1. The refusal path of a composed ladder, until ladder.py:123 is fixed — and afterwards, that a layered rung is built on the resolver's handling rather than on its own try/except, or the (source, reason) pairs flatten again.
  2. A rung can still fall back internally and the ladder cannot see it — carried from round 1 and still true. The species axis is the last line of defence; every M1 source must be honest about its own species even when it answers.
  3. The projection type and its import guard, per the rewording above.
  4. Which granularity a refused-fraction threshold is against. refused_steps is any term refused; refused_seconds is only refused terms. A run can be 100% refused by steps and 4% by seconds. Still unstated.
  5. Do not subclass StepCost — and if the shadow set on cost.py:137 is not widened, know that is_refused and refusals are shadowable and that hiding them makes a run report refused_step_fraction == 0.0 over steps that did refuse.

Not checked

  • The GPU tier. GATE_CPU_RC=0 and gpu: not required on all four trees; I did not run it and make no claim about it.
  • Concurrency. ProvenanceMix is still not thread-safe and nothing here says it must be.
  • Whether Tier's three values are the right partition. Nothing consumes them; declared-not-checked is already recorded as left undone.
  • The merged tree's lint baseline. I ran the CPU gate on it, not ruff across the union.

Re-review on push. The verdict turns on ladder.py:123 alone.

…#23)

Review found a shape with no correct implementation. A rung that is itself a
ladder has to call `resolve`, and when its inner ladder exhausts, the
`CostRefused` propagated straight out of the outer `resolve`: the outer
ladder's own refusals were discarded and every rung below the composed one was
never consulted. A run could refuse where it had an answer available, and
could not say what it had passed over -- the silent fall-through inverted. The
module's claim that an exhausted ladder "carries every refusal in order" was
false under composition.

The rung call is now wrapped: an inner exhaustion extends this ladder's
refusals with the inner chain and consults the next rung. The inner pairs stay
keyed by the inner rungs that actually declined rather than being flattened
onto the wrapper, so the reason counts still name something an operator can go
and measure. Verified: with an inner ladder that exhausts and a lower rung
that can answer, the lower rung answers and all three refusals arrive in
order; with nothing able to answer, the raised list names the inner rungs too.
A source that raises with an empty list is attributed to the rung itself
rather than vanishing.

The subclass shadow set was three names and needed to be every reader. Hiding
`is_refused` makes a run report no refused steps over steps that did refuse,
with no bypass and through the public API, at the layer that reads that
fraction to decide whether the run is evidence -- the same defect as the empty
breakdown and the empty run, arriving by a third route. The set is now read
off the class rather than listed, so a reader added later is covered the day
it is added. Seven readers, seven refusals.

`seconds_by_species` says what it guarantees: each group is the fold of its
own terms in stored order, and grouping re-associates, so folding the group
totals is a different sum from `seconds` and is not bit-equal to it. `rows()`
is the view that re-folds to the total. A test pins a case where the two
differ rather than leaving it to a reader to discover.

The engine-import scan now walks the package recursively, and the note about
the batch view is written as an obligation on whoever defines it: the property
belongs to a package, not to a type, and a projection defined outside this one
is covered by nobody until its own package's tests assert the same thing.

`atom/compass/__init__.py` is taken verbatim from the integration branch,
which landed its own copy while this was in review. That removes the add/add
conflict rather than leaving it for the landing.

Tests: 68 cases, up from 54.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Round-2 blocking finding and all three secondary ones are fixed in 344c19e89, pushed on top of 1e03d3979 — no force-push. Body updated; every gate figure now names its commit.

Blocking — resolver-backed rung. Your diagnosis was exact, including why the obvious alternative is also wrong. The rung call is now wrapped, and an inner CostRefused extends this ladder's refusals with the inner chain and continues. I kept the inner pairs keyed by the inner rungs rather than flattening them onto the wrapper, for the reason you gave: a reason count that names the wrapper names something nobody can go and measure.

Verified rather than pasted, with your Layered helper:

  • inner exhausts, outer has a lower roofline rung → roofline answers, declined == [price_list, inner_price_list, nearest_key], provenance carries all three, reasons() counts three distinct pairs;
  • everything exhausts → the raised declined lists the inner rungs too, so the module docstring's claim is now true under composition rather than trimmed;
  • a source that raises CostRefused(request, ()) is attributed to the rung itself (silent_layer) rather than vanishing — a hand-rolled source can do that and silence would be a fall-through with nothing recorded.

Three tests, plus one on the exhaustion list.

Shadow set. You were right that it was not cosmetic. It is now derived from the class — vars(StepCost) plus its annotations — rather than listed, so a reader added later is covered on arrival instead of when someone remembers. Seven today; all seven refused, asserted by a parametrised test over the derived set so the assertion grows with the class. test_hiding_the_refusal_flag_would_empty_the_refused_count states the defect as the thing it breaks: the mix reads refused_step_fraction == 1.0 on that step, and the subclass that would have made it 0.0 is refused at class definition.

Grouping. Stated rather than routed — routing is not available, since grouping re-associates by definition. seconds_by_species now documents that each group is the fold of its own terms in stored order, that folding the group totals is a different sum and not bit-equal to seconds, and that rows() is the view a total is checked against. A test pins a concrete divergence (0.1 / 0.2 / 0.15, first and third sharing a species: flat 0.45000000000000007, grouped 0.45) rather than leaving a reader to find one.

W1.6 note. Agreed it was vacuous. The glob is now rglob, and the note is rewritten as an obligation and moved into base.py, where the projection's author will read it: the no-engine-imports property belongs to a package, not to a type; this test reads only files under backends/; a projection defined anywhere else is covered by nobody until its own package's tests assert the same thing over its own sources.

Your retraction noted — thank you for re-running it rather than re-reading it.

Gates — four trees, every figure named. Node 18, xiaobizh_n18_cpu, git archive snapshots md5-verified host → node → container.

Tree Commit Passed Skipped xfailed rc
This branch's base c19710bcd 4030 149 3 0
Integration head f67618eb9 4084 149 3 0
This branch 344c19e89 4098 149 3 0
Merge of the two 7b6abcfd1 4152 149 3 0

4030 + 68 = 4098 and 4084 + 68 = 4152; 68 decomposes 29 + 15 + 15 + 9. All four read 149 skipped, so the one-in-four 4029/150 flake did not appear and no re-run was needed. I reproduce your 4138 as 4084 + 54, the previous branch state.

The merge is now clean. I took atom/compass/__init__.py verbatim from f67618eb9, so the add/add conflict is resolved on this branch instead of at landing. gpu: not required on all four trees. ruff and black clean on all ten files.

AST lines 261 (227 → 256 → 261), inside the 200–300 estimate: cost.py 109, ladder.py 68, provenance.py 56, base.py 22, inits 5 + 1. By AST enumeration the package has exactly five Add nodes — one in fold_seconds, three integer counters, one tuple concat — matching your count.

Still draft, not merged, #23 still open.

# refusals are this ladder's refusals -- keyed by the inner
# rungs that actually declined, not flattened onto the wrapper,
# so the reason counts still name something measurable.
declined.extend(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The blocking round-2 finding is closed, and I could not break it. Two unvalidated payloads remain on this new branch — non-blocking, both reachable only from a hand-rolled source.

I attacked the control flow rather than the data, at 344c19e89. Everything the fix claims reproduces:

Composition Result
depth 6, one decline per level, bottom ladder exhausts, roofline answers at level 1 source=roofline, declined=[d1,d2,d3,d4,d5,d6a,d6b], chronological, provenance identical
the same six levels with nothing answering raised list = the same 7, 7 distinct pairs, none duplicated
dead wrapper, live wrapper and plain rungs at one level declined=[top,j_price,j_near], provenance adds i_price last, which is where it happened; reasons() = 4 distinct pairs keyed by the inner rungs
wrapper first / wrapper last / exhaust-everything order chronological in each
deep answer path at 6 levels name composes w2/w3/w4/w5/w6/a6, chain d1..d6
CostRefused raised from a rung's construction rather than an inner resolve caught here and treated as a decline, declined=[('cfg','no price list configured')]
a memoising rung that caches the term it resolved, over 3 steps chain stays [o_price, i_price], does not accumulate — Provenance being frozen is what buys that, and W1.6 will lean on it

I found no path where a refusal is counted twice for one consultation, and no path where the chain stops being chronological. The two cases that look like double counting are not: the same inner ladder wrapped under two rung names is genuinely consulted twice (reasons() -> {(s_price,…): 2}), and an inner rung sharing a name with an outer rung stays distinguishable by reason. Keeping the inner pairs keyed by the inner rungs is the right call and it holds under nesting.

Two things this except branch does not check that the return path below it does.

1. source.refuse(...) on the empty-list branch is not validated. Lines 150-155 reject a returned refusal that names somebody else; this branch does not:

class SilentLiar(CostSource):
    name = "liar"
    def refuse(self, reason): return Refusal("somebody_else", reason)
    def price(self, request): raise CostRefused(request, ())

Resolver([SilentLiar(), Answers("roofline", 0.009)]).resolve("attn")
  declined -> [('somebody_else', 'its own sources refused, naming none')]

Same override on the return path is refused: ValueError: source 'liar2' returned a refusal naming 'somebody_else'; a refusal has to name who declined. CostSource.refuse's docstring says "so it cannot name another", and on this one branch it can — because the fallback goes through the overridable helper instead of constructing the refusal the resolver already knows is correct. Refusal(source.name, "its own sources refused, naming none") is the same length and cannot be overridden.

2. CostRefused.declined is not type-checked, and it is now load-bearing control flow. CostRefused.__init__ does tuple(declined) and nothing else. A rung that raises with junk splits two ways:

class Junk(CostSource):
    def price(self, request): raise CostRefused(request, ["just a string"])

Resolver([Junk(), Answers("roofline", 0.009)]).resolve("attn")
  -> TypeError: not a refusal: 'just a string'     # raw, out of resolve, from Provenance

Resolver([Junk()]).resolve("attn")
  -> CostRefused.declined == ('just a string',)
     message: "no cost source answered for 'attn': just a string"

On the refusal path a non-Refusal reaches the caller's declined tuple and the message unchecked — so reasons() downstream unpacks a string, and the coverage report carries an entry nobody can group. The return path is type-checked (not isinstance(answer, CostTerm) names the source); the raise path was not previously control flow and now is. One isinstance loop in CostRefused.__init__ closes both halves, and closes them at the constructor rather than at each catch site.

Neither is blocking: both need a source that either overrides refuse or hand-rolls the exception, and no number is invented on either path. Flagging them because they are the same asymmetry round 2 closed on the return path, now on the branch that replaced it.

somebody remembers to extend a tuple.
"""
super().__init_subclass__(**kwargs)
owned = {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

The derivation does what round 2 asked: every route by which a reader can be added to this class lands in the set. Measured, 6 of 6.

I added a probe_reader to StepCost by each route in turn and asked whether the derived set picked it up and whether a subclass shadowing it was then refused:

Reader added to StepCost by in derived set subclass refused
functools.cached_property yes yes
a custom (non-data) descriptor yes yes
a plain method yes yes
classmethod yes yes
staticmethod yes yes
__annotations__ only, no value yes yes

vars(StepCost) is the class __dict__, so anything bound on the class is in it whatever wrapper it arrives in, and the annotations arm covers the one route that binds nothing. The seven today are is_refused, refusals, refused_seconds, rows, seconds, seconds_by_species, terms, and the parametrised test refuses all seven. This is strictly better than the tuple and I have nothing to add to it.

Three routes it does not cover. Reporting them measured, not as asks — the first two are acts beyond defining a subclass, which is the category you already reserved.

  1. Assignment after the class body. __init_subclass__ runs once, at creation:
K = type("F", (StepCost,), {})          # accepted
K.seconds = property(lambda s: 99.0)
K([CostTerm("a", 0.1, P)]).seconds -> 99.0
  1. __getattribute__ interception. Underscored, so filtered out of owned, and it is not a StepCost name to begin with — but it reproduces the exact defect the set exists to stop, with a plain subclass and no bypass primitive:
class G(StepCost):
    def __getattribute__(self, n):
        if n == "is_refused": return False
        if n == "seconds":    return 99.0
        return object.__getattribute__(self, n)

mix.record(G([one refusing term]))
  mix.refused_step_fraction -> 0.0
  1. A reader inherited rather than owned. vars(StepCost) is not StepCost.__mro__. StepCost has only object above it today, so this is latent, not live — but the day a mixin carries a reader, a subclass shadowing that reader is accepted. If you ever add a base, the derivation wants to walk the MRO.

None of these blocks. (1) and (3) are hypothetical today, and (2) is in the same class as object.__setattr__, which the module already declines to chase. Worth one sentence in the docstring saying what the seal covers — a subclass that defines a reader — so the next reader does not take it for a guarantee that a subclass cannot lie at all.

)
return self.refused_seconds / self.seconds

def seconds_by_species(self) -> Mapping[Species, float]:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Non-blocking, principle 7 — the grouping statement is right, and it stops one layer below the layer that reads it.

StepCost.seconds_by_species now documents re-association and pins 0.45000000000000007 against 0.45, and the test holds at 344c19e89. Accepted: re-association is definitional and stating it is the right remedy.

This method — ProvenanceMix.seconds_by_species, the run-level one — has no docstring, and the same divergence is there:

one step, terms 0.1/0.2/0.15, first and third sharing a species
  ProvenanceMix.seconds                 -> 0.45000000000000007
  fold_seconds(mix.seconds_by_species()) -> 0.45

4000 random runs (1-5 steps, 1-4 terms each, two species):
  mix.seconds != fold_seconds(mix.seconds_by_species().values())  in 1044 of 4000

The reason it matters more here than one level down is that StepCost has rows() — a reader with the artifact has a decomposition that does re-fold to the total, and the mixture is an extra view beside it. ProvenanceMix has no rows(). At the run layer the mixture is the decomposition a reader gets, and it does not add up to the total printed beside it about a quarter of the time. That is the layer the admissibility fraction is read at. Same sentence, one method up, and the artifact task does not have to rediscover it.

Also still open from round 1, and reproducing — recording it because this is the third cycle and it should stop being carried silently, not because it blocks:

mix.record(StepCost([CostTerm("a", 0.1, M), CostTerm("b", 0.2, M)]))
mix.refused_steps = 99
  mix.refused_step_fraction -> 99.0
mix.seconds = 0.0
  mix.seconds -> 0.0   mix.seconds_by_species() -> {measured: 0.30000000000000004}

steps, seconds, refused_steps and refused_seconds are the only numbers in the package with no invariant on them, and refused_step_fraction is not even range-checked, so the admissibility layer can be handed 99.0 as a fraction. StepCost is sealed against considerably less than this. I am not asking for it in this PR — the gap is between a frozen value type and a mutable accumulator, and sealing the accumulator is a different shape of work — but whoever builds the artifact writer should either make the four counters private with seconds folded from _species_seconds, or say in the docstring that these are a caller-maintained tally.

The two empty-run refusals are correct and the denominators really are split: an all-free run answers refused_step_fraction == 0.0 and refuses only the seconds fraction, which is the round-1 ask exactly.

raise ValueError(f"{self.name}: {self.seconds} is not a finite duration")
if self.seconds < 0.0:
raise ValueError(f"{self.name}: {self.seconds} s is negative")
if self.provenance.is_refused and self.seconds <= 0.0:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Accepted as your call, with the reservation stated: the round-2 collision is resolved in prose, not in code, and the exhaustion fix widened where it can be reached from.

The PR body settles it — "a zero-valued term is allowed when nothing refused" — which is the second horn of the round-2 comment, and it is a defensible choice: a rung that prices a refusal at nothing is a protocol violation, and this constructor is where protocol violations are refused. The rest of the diff is consistent with that reading.

What did not change is the shape of the failure, and the new except CostRefused branch gives it a second route in:

Resolver([Declines("price_list", "no entry"),
          Answers("free_law", 0.0)]).resolve("collective")
  -> ValueError: collective: a refusal was priced at 0.0 s. ...

Resolver([Answers("free_law", 0.0)]).resolve("collective")
  -> 0.0            # same rung, same answer, no fall-through

inner = Resolver([Declines("i1", "a"), Declines("i2", "b")])
Resolver([Layered("lay", inner), Answers("free_law", 0.0)]).resolve("collective")
  -> ValueError     # new at 344c19e89: an inner exhaustion now also arms it

So: Resolver.resolve exits with a raw ValueError that is neither an answer nor a refusal, on an input that is ordinary — "no collective on this step" is the PR body's own example of a legal zero, and a zero term below any decline is now also reachable through a composed rung that exhausted. The message misdescribes it too: nothing stood in, the rung answered its true cost.

Not blocking, and I am not asking for a fourth round on it. It sits in the same class as the other protocol violations this module raises out of resolve (a source returning a non-term, a refusal naming a neighbour), a backend can avoid it by omitting an absent term rather than pricing it at zero, and you stated the rule. This is the item W1.6 will meet first, so it wants to be a line in the handoff rather than a discovery: either the first tier-b backend omits zero terms, or Resolver converts this one constructor error into a Refusal naming the answering rung.

@jgong5

jgong5 commented Sep 21, 2026

Copy link
Copy Markdown
Owner Author

Review — cycle 3 of 3, c19710bcd..344c19e89 (head at posting: 344c19e89)

Verdict: APPROVE. The loop converges here — it did not halt, and no need human label is applied.

Agent-authored. Everything below was measured on this branch; the gate was re-run from scratch on all four trees rather than read off the body.

The blocking round-2 finding is closed, and I could not break it

Round 2 established there was no correct way to write a resolver-backed rung. The fix — catch, extend with the inner chain keyed by the inner rungs, continue — is the right shape, and it survived every composition I could build at 344c19e89:

  • depth 6, one decline per level, bottom ladder exhausting, roofline answering at level 1: declined == [d1,d2,d3,d4,d5,d6a,d6b], chronological, provenance identical;
  • the same six levels with nothing answering: the raised list is all 7, 7 distinct pairs, none duplicated;
  • a dead wrapper and a live wrapper beside plain rungs at one level: the live inner ladder's own decline arrives after the dead one's two, which is where it happened, and reasons() is four distinct pairs all naming rungs somebody can go and measure;
  • CostRefused raised from a rung's construction rather than an inner resolve: caught and treated as a decline;
  • wrapper-first, wrapper-last, exhaust-everything: chronological in each;
  • the answer path at six levels: w2/w3/w4/w5/w6/a6, chain d1..d6;
  • a memoising rung that caches the term it resolved, over three steps: chain stays [o_price, i_price] and does not accumulate. Provenance being frozen is what buys that, and it is worth W1.6 knowing it is load-bearing.

No refusal is counted twice for one consultation, and I found no path where the chain stops being chronological. The two shapes that look like double counting are not: the same inner ladder wrapped under two names is genuinely consulted twice, and an inner rung sharing a name with an outer rung stays distinguishable by reason.

Two unvalidated payloads remain on the new branch — a refuse() override that can name a neighbour on the empty-list fallback, and CostRefused.declined accepting non-Refusal entries. Both are non-blocking, both need a hand-rolled source, and both are one line each; detail inline on ladder.py.

The derived shadow set

Round 2's ask is met and then some. I added a reader to StepCost by six routes in turn — cached_property, a custom descriptor, a plain method, classmethod, staticmethod, __annotations__-only — and all six land in the derived set and all six are then refused on a subclass. vars(StepCost) catches anything bound on the class whatever wrapper it arrives in; the annotations arm catches the route that binds nothing. Three routes it does not cover — post-hoc K.seconds = property(...), __getattribute__ interception, and a reader inherited from a base StepCost does not have today — are measured inline; none blocks.

Gates, re-measured, every figure named

Four git archive snapshots built with both stamps written from the same rev-parse (so no exit 98), md5-verified host → node 18 → xiaobizh_n18_cpu, gate_cpu.sh run in each:

Tree Commit Passed Skipped xfailed GATE_CPU_RC gpu
branch base c19710bcd 4030 149 3 0 not required
integration head f67618eb9 4084 149 3 0 not required
this branch 344c19e89 4098 149 3 0 not required
merge 7b6abcfd1 4152 149 3 0 not required

Every number in the body reproduces. 4030 + 68 = 4098, 4084 + 68 = 4152; 149 skipped on all four, so the flake did not fire. The 68 decompose exactly as claimed — collected per file: step_cost 29, provenance 15, ladder 15, interface 9.

The merge is clean and reproducible. git merge-tree --write-tree f67618eb9 344c19e89 returns b3d15a8c0 with no conflict, which is byte-for-byte the tree of 7b6abcfd1, whose parents are 344c19e89 and f67618eb9. atom/compass/__init__.py on this branch is md5-identical to f67618eb9's copy, so the add/add conflict really is resolved here and not deferred.

Effort: 261 production AST lines, counted with my own parse/strip-docstrings/unparse pass — cost.py 109, ladder.py 68, provenance.py 56, base.py 22, backends/__init__.py 5, compass/__init__.py 1. Tests 327 by the same method. Both match the body exactly. Five Add nodes in the package and no sum() anywhere: one in fold_seconds, three integer counters, one tuple concatenation.

ruff check and black --check clean on all ten files. No design-doc reference in any source, test, docstring or emitted string — grepped for decision ids, section marks, document filenames, wave and task ids: none. The package's own modules import nothing outside atom.compass.backends (the rglob scan is genuinely recursive now, five modules, five cases); import atom.compass.backends pulls in atom.plugin.sglang and atom.sampling_params from atom/__init__.py above it and does not import torch, so the no-driver claim holds where it is used.

Accepted with reservation — named, and needing no further review

  1. The grouping statement stops one layer short. StepCost.seconds_by_species documents re-association and pins the divergence; ProvenanceMix.seconds_by_species has no docstring and the same divergence is there — 1044 of 4000 random runs have mix.seconds != fold_seconds(mix.seconds_by_species().values()). It matters more at the run layer because there is no rows() there: the mixture is the decomposition a reader gets. One sentence, the one you already wrote.
  2. ProvenanceMix's four public counters still have no invariant (round 1, restated round 2). mix.refused_steps = 99 gives refused_step_fraction == 99.0 — not range-checked, at the layer that reads it to decide whether a run is evidence. Non-blocking both prior cycles and non-blocking now; it is a frozen-value-type seal against a mutable accumulator, which is different work.
  3. The zero-priced-refusal collision is settled in prose rather than in code, and the exhaustion fix gave it a second route in: after an inner ladder exhausts, a legitimate zero term below it raises ValueError out of resolve, which is neither an answer nor a refusal. Your call is defensible and stated; I am not opening a round on it.

None of the three needs another review pass. Items 1 and 3 belong in the W1.6 handoff.

What W1.6 should watch

  • The zero term. "No collective on this step" is the first shape that meets item 3. Omit absent terms rather than pricing them at zero, or convert that one constructor error into a Refusal naming the answering rung.
  • The projection's package owns the no-engine-imports assertion. The obligation in base.py is the right remedy and it is not vacuous — nothing in this package can assert a property of another one — but it is prose, and it is only discharged when the package that defines the projection asserts the same thing over its own sources.
  • The seal covers a subclass that defines a reader. It does not cover a class attribute assigned afterwards or __getattribute__. Do not read it as "a StepCost subclass cannot lie".
  • Memoising a resolved term is safe because Provenance is frozen. If a future rung ever mutates a provenance in place, the chain accumulates across steps.
  • test_the_package_imports_nothing_from_the_engine is parametrised over this package's files, so the CPU pass count moves by one per module added under backends/. Already flagged in the body; it is the one number in this wave that is not a constant.

Could not check

The GPU tier — gate_cpu.sh reports gpu: not required from the .compass-changed stamp on all four trees, and no path in the diff matches gpu_gate_triggers.txt, so no claim is made about it either way. Tier is declared and not verified against artifacts, which is correctly out of scope here.

PR #58 is ready to land. This is cycle 3 of 3 and the loop converged on approval rather than exhausting; need human is not applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant