fix(nowcast): hold the 336414/33641A see-saw the 2022 census invents (#862)

[WesIngwersen]

Closes #862. Stacked on #873, at the top of the trade/margins stack.

The evidence needs no threshold

336414 and 33641A are one see-saw, and the decisive fact is that the census and BEA agree almost exactly on the pair's combined output and disagree entirely about the split:

2022 gross output, $M	336414	33641A	pair
BEA, unconditioned	22,001	20,849	42,850
census-conditioned	16,879	25,982	42,861

The total agrees to 0.03% while 5.1 billion USD moves between the two members. That is a reallocation, not a measurement of growth — and guided missiles is concentrated enough that one firm's classification between vehicles and propulsion units moves the published split with no economic change behind it.

QCEW payroll is opposed in both halves: the census moves 336414 to 0.744× BEA and payroll to 1.226×; 33641A 1.209× against payroll's 0.766×.

What holding the pair buys

	shipped	pair held
336414 surplus, 2022	−2,957	+311
336414 surplus, 2024	−3,693	−22
economy-wide negative surplus, 2022	−20,952	−17,995

That is #862's headline, and the band BEA's own unconditioned series sits in.

❌ They are held as a pair. Holding 336414 alone imposes 33641A's move and changes the family's split in a way neither source supports — which is #862's own open decision 1, answered.

⚠️ No automatic screen is shipped, and that is the considered answer

#862's decision 2 asks whether the screen should look at more than the coverage wedge. I built two constant-free rules and both fail:

❌ Sign-flip on gross operating surplus — hold where the conditioning drives surplus negative and BEA's own series does not. Fires on zero industries in every year. 336414 is negative under both arms; the conditioning changes its magnitude roughly tenfold without crossing zero. There is no accounting-boundary rule available here.

❌ Rank by census–payroll disagreement — dominated by payroll noise rather than census error. The largest disagreement of all 223 industries is 333991 handtools, where the census matches BEA to 4.9% and payroll runs +98.5%. 94 of 223 industries disagree in direction; the see-saw sits at 0.481 against a 99th percentile of 0.681 and does not stand out at all.

⚠️ An earlier draft of this PR used a gate on the size of the census move — the arbitrary constant this project is trying to remove. It was not derivable from anything: 0.25 flags one industry, 0.20 three, 0.15 six, 0.10 sixteen. Worse, 0.25 splits the see-saw — holds 336414, imposes 33641A — the one outcome both sources agree is wrong. The gate is gone; the decision lives in PENDING_REVIEW, a list of named industries with measured reasons.

What is in this PR

⚠️ Three unrelated changes, and the title only covers one. Review caught that the original description listed two files while the diff has four. Closes #862 and #867.

1. The see-saw hold (#862)

• ec_go_adjustment.PENDING_REVIEW gains 336414 and 33641A, documented as one see-saw held together and why.
• analysis/nowcasting/ec_payroll_corroboration.py — the three growth series ranked and unfiltered, for a human reviewing a census move. --check asserts all nine figures above.
• family_reallocation() — a first cut at the generalisation this points at: which BEA groups does the census reallocate while leaving the group total alone. That is the shape of the 3364 defect and needs no magnitude constant.

2. ⚠️ The value-added residual now routes to a row the industry actually has — nowcast.py

This has the widest blast radius in the PR: it changes every industry's value-added block in every year, where the see-saw hold changes two aerospace codes. It was previously invisible in this description.

_reconcile_to_published_vapro routed every industry's reconciliation residual to V00300 gross operating surplus. Two industries have no surplus row at all. BEA publishes V00300 of exactly zero for 814000 in 2007, 2012 and 2017, with VAPRO equal to compensation to the dollar — private households earn no operating surplus, definitionally, because the sector's output is the compensation it pays. 4200ID is the same shape one row along: a synthetic code carrying customs duties.

This was a hard blocker, not a rounding matter. The routing put 1,967 $M (2018) rising to 5,104 (2024) into a cell the balance mask holds as a Tier 0 structural zero, and seven of the eight years failed outright. value_added_residual_row() now derives the absorber per industry from the published 2017 table; exactly two industries take the exception.

Pinned by a new test rather than by prose — review's catch, and a fair one, since the result was asserted in the docstring and held nowhere. The derivation's bare 0.5 is now VA_ROW_PRESENCE_ATOL with its reasoning stated (BEA publishes in millions and rounds to the unit; same bar as sut_use_to_mut_use.REPLAY_ATOL). A third industry newly qualifying now fires the test instead of being picked silently by idxmax.

3. The staleness checker (#867) — stale_artifacts.py

builder is resolved as a dotted module and used as the method surface, plus a new unverifiable class for artifacts carrying neither lineage nor a resolvable method, and --delete / --scope flags.

Review found four problems in it, all fixed here:

• The module docstring claimed unverifiable catches the Step 5-7 products. It does not — method_files resolves their builder in its first branch, so they land in method. Measured on this tree: --name Nowcast_Detail reports 108 method changed, 0 unverifiable. The docstring now says what the class actually catches (mostly flowsa's FBAs) and where the Step 5-7 hole is really closed.
• --delete had no dry run. It is now a dry run listing what would go, with --yes to act, and it never deletes unverifiable artifacts — "cannot be checked" is not "known stale", and a Step 5 or 6 product removed on that basis costs hours to rebuild.
• --delete --strict deleted and exited 0, the two joined by elif. They are independent now: it deletes and still exits 1.
• _tracked_change_times needs full history. A shallow clone (fetch-depth: 1, the Actions default), a failed subprocess and an expired timeout all returned {} silently, which reads as "nothing is stale". All three warn on stderr now, and the docstring says to fetch full history before gating CI on --strict.

Left on the table, deliberately

325120 industrial gases is opposed too — census 1.237× BEA, payroll 0.902× — and is not held: its surplus is comfortably positive either way (4,273 million at 2022). A census move nothing corroborates, but no evidence of harm. Acting on it would be acting on the screen rather than on the symptom.

Testing

Covering all four files, not just the two the original block ran:

# 1. the see-saw hold
uv run python -m bedrock.analysis.nowcasting.ec_payroll_corroboration --check  # 9/9 PASS
uv run pytest bedrock/transform/iot/__tests__/test_ec_go_adjustment.py -q      # 6 passed

# 2. the value-added routing
uv run pytest bedrock/transform/iot/__tests__/\
test_derived_intermediate_and_value_added.py -q     # 16 passed, absorber pinned
                                                    # to {814000: V00100,
                                                    #     4200ID: T00TOP}

# 3. the staleness checker, behaviour that had no test
uv run python -m bedrock.utils.validation.stale_artifacts --name Nowcast_Detail
    # 108 problems: 108 method changed, 0 unverifiable
uv run python -m bedrock.utils.validation.stale_artifacts --name Nowcast_Detail --delete
    # DRY RUN -- lists, deletes nothing, exit 0
uv run python -m bedrock.utils.validation.stale_artifacts --name Nowcast_Detail --delete --strict
    # exit 1  (was 0 before this PR)

uv run black --check . ; uv run ruff check . ; uv run mypy bedrock             # clean

⚠️ This changes gross output for two industries in every year 2022+, so it obliges a rebuild of the value-added block and everything downstream.

🤖 Generated with Claude Code

[bl-young]

The diff carries two changes the description does not mention.

The "What is in this PR" list covers ec_go_adjustment.py and ec_payroll_corroboration.py. The diff has four files, and the other two are both load-bearing:

• nowcast.py (+63/-1) adds value_added_residual_row() and rewrites _reconcile_to_published_vapro to route each industry's reconciliation residual to the value-added row it actually carries. This is the 814000 household gross-operating-surplus fix -- the one docs(nowcast): the 7 September rebuild — all eight years, and what the stack moved #875 refers to as "this PR's parent" and credits with unblocking seven of the eight years.
• stale_artifacts.py (+322/-3) adds two staleness classes (method, unverifiable), a --delete flag and a --scope flag. That is the work for stale_artifacts cannot see a method change, so an edited FBS method silently returns the previous build #867.

Each of the three is a good change and I am not asking for any of them to be dropped. But the see-saw hold, the value-added routing fix and the staleness checker have nothing to do with each other, and only one of them is reviewable from the description. The Testing block has the same gap: it runs test_ec_go_adjustment.py and the corroboration --check, neither of which touches nowcast.py or stale_artifacts.py.

Please update the description and the Testing section to cover all four files, and reference #867 the same way #862 is referenced. The value-added routing fix in particular needs its own paragraph: it changes every industry's value-added block in every year, which is a much wider blast radius than holding two aerospace codes, and right now a reader would not know it is in here at all.

[bl-young]

The routing fix has no test, and its failure mode already cost seven of eight years.

bedrock/transform/iot/__tests__/ has no reference to value_added_residual_row or _reconcile_to_published_vapro. That is the one place in this diff where I think a test is genuinely owed rather than paranoid, for three reasons:

1. The failure mode is recorded history, not hypothesis. The docstring says it plainly: seven of eight years failed outright with "1 cells are nonzero in the seed but marked structural zero" because the residual was routed to V00300/814000. A regression here does not raise -- it either fails the balance a rebuild later, or routes billions to a row that should be zero and lets it into every downstream coefficient.

2. The result is asserted in prose but pinned nowhere. The docstring states that exactly two industries take the exception and names them (814000 -> V00100, 4200ID -> T00TOP). Nothing in the code or the tests holds that. It is derived at runtime from the published 2017 table.

3. The derivation is not obvious from reading it. An industry keeps V00300 if abs(block.loc['V00300', industry]) > 0.5; otherwise the absorber is carried.idxmax(), the largest-magnitude row it does carry. That 0.5 is a bare constant, and idxmax() will silently pick some row for any industry that newly qualifies -- there is no "I do not know" branch. Worth noting the irony that this PR's own argument against the census-move gate is that arbitrary constants are what the project is trying to remove.

Suggested test, one failure mode, no restating of the implementation:

def test_only_two_industries_take_the_non_surplus_absorber() -> None:
    """814000 and 4200ID carry no gross operating surplus; everything else does.

    Routing a residual to V00300 for these two puts value in a cell the
    balance mask holds as a structural zero.
    """
    absorber = value_added_residual_row()
    exceptions = absorber[absorber != 'V00300']
    assert exceptions.to_dict() == {'814000': 'V00100', '4200ID': 'T00TOP'}

If the published table is ever revised such that a third industry qualifies, that fires and a human looks at it, which is the outcome you want. It is also cheap -- the function is functools.cached and reads a table the suite already loads.

[bl-young]

stale_artifacts.py: unverifiable does not do what its docstring says, and --delete has no safety.

Four things in this file, in descending order of how much they matter.

1. The unverifiable class documents a case it does not actually catch. The module docstring introduces unverifiable with the Step 5 and Step 6 products as its motivating example -- "written by bespoke savers that record a builder and an engine_result but no sources", and "before this class existed the checker answered 'No stale cached artifacts' about balanced SUTs built weeks earlier than the trade inputs underneath them."

But method_files() resolves builder in its first branch and returns the module path, so files is non-empty for exactly those artifacts. The unverifiable branch requires not files, so the balanced SUTs and the MUT quartet never reach it -- they land in method instead, flagged when their builder module moves. method_files()'s own docstring describes this correctly ("Resolve the builder so at least a change to it is visible"); the two accounts disagree.

That is worth fixing because the module docstring is what a reader consults to decide whether a flag is real. It should either describe what unverifiable actually catches (artifacts with no builder, no cornerstone-data/bedrock method_url, and no sources -- flowsa's FBAs, mostly) or the resolution order should change. Note also that routing them to method does not close the original hole: a change to the trade inputs underneath a balanced SUT still moves nothing that this check can see, because the builder module did not change. That is the same gap #875 flags as remaining, so the class as written does not fix it and the docstring implies it does.

2. --delete has no dry run and no confirmation. It removes the parquet and the metadata for every flagged artifact in scope, defaulting to transform. #875 reports 386 flags standing after the rebuild, including 108 superseded versions of the nowcast quartet which live in transform/output_data. Everything is recoverable from GCS, but a one-word flag that silently takes out a Step 5 or Step 6 product costs hours of rebuild to undo. Suggest either defaulting to a dry run with an explicit --yes to act, or -- probably better -- excluding the unverifiable class from deletion on the grounds that "cannot be checked" is not "known stale."

3. --delete silently swallows --strict. They are joined by elif, so --delete --strict deletes and exits 0. If this ever becomes a pre-rebuild CI gate, that is a false pass rather than a loud one.

4. _tracked_change_times() needs full history. It parses git log --name-only over the whole repo. Under a shallow CI clone (fetch-depth: 1, which is the common default) almost no path gets a commit time, so method staleness under-reports silently instead of erroring. Worth a note in the docstring at minimum, and worth knowing before --strict gates anything in Actions. The 180s timeout returns {} on expiry too, which fails the same open way.

The design itself is sound -- the mtime half gated on _dirty_paths() content rather than raw mtime is exactly right, and the note about the first version flagging 985 of 995 artifacts is a good record of why the narrow resolution is the correct one.

[bl-young]

Approving this despite the comments above. I'd recommend they be addressed soon after we pull in this stack (if not before). But I would recommend updating the PR body at minimum.

[WesIngwersen]

All three addressed. Thank you — the first and third were both cases where the document and the code disagreed, which is the worst kind to leave standing.

1. The description covered two of four files. Rewritten into three numbered sections, and it now closes #867 the same way it closes #862. The value-added routing has its own section saying plainly that it has the widest blast radius in the PR — every industry, every year, against two aerospace codes for the see-saw. The Testing block runs all four files now, including the staleness behaviour that had no check at all.

2. The routing fix had no test. Added yours, essentially verbatim — it passes and pins {814000: V00100, 4200ID: T00TOP}. You were also right about the bare 0.5: it is now VA_ROW_PRESENCE_ATOL with the reasoning stated (BEA publishes in millions and rounds to the unit, so half a unit is where "carries no such row" stops being a rounding artefact — the same bar as REPLAY_ATOL). Fair hit that a PR arguing against arbitrary constants had introduced one. I left idxmax as it is: the test is the "I do not know" branch, since a third industry qualifying now fires it instead of being picked silently.

3. stale_artifacts, all four. Your read of the resolution order was exactly right, and measurable — --name Nowcast_Detail reports 108 method changed, 0 unverifiable, so those products never reach the class the docstring credited. Docstring now describes what it actually catches and points at where the Step 5-7 hole is really closed.

• --delete is a dry run; --yes acts. It also never deletes unverifiable artifacts — your framing that "cannot be checked" is not "known stale" is the right rule and I took it as written.
• --delete --strict now exits 1. It exited 0 before, exactly as you said.
• _tracked_change_times warns on stderr for all three silent paths (shallow clone, failed subprocess, expired timeout), and the docstring says to fetch full history before --strict gates anything.

On your closing point in 3.1 — that routing them to method does not close the original hole — that is right and it is now fixed one PR up rather than papered over: utils.metadata.source_lineage records what a saver actually read, wired into save_mut and save_after_redef. Step 5 still cannot name its inputs and #875 says so explicitly.