Skip to content

parity-governance: a stale default base hides that the authority refresh needs issue-bound dispositions #2229

Closed

Description

@ryanbr

parity-governance is red on main, and the refresh paths that exist to fix it both refuse.

Current state

main at 071f432d2 fails two RepositoryBaselineTests:

test_checked_in_inventory_and_baseline_match_current_sources
test_checked_metadata_is_compact_v3_and_expands_losslessly

AssertionError: {'files': {'count': 490, ...}} != {'files': {'count': 493, ...}}

The checked-in authority in Tools/parity_twin_map.json records 490 scanned files. The tree actually has 493. Confirmed independently: parity_ledger._paths returns 263 Swift and 230 Kotlin, and 263 + 230 = 493.

What drifted

4453a089b (Lift Log) added three files that the Swift globs scan, without refreshing the authority:

Packages/StrandAnalytics/Sources/StrandAnalytics/LiftMetrics.swift
Packages/StrandImport/Sources/StrandImport/LiftProgramSheetImporter.swift
Packages/StrandImport/Sources/StrandImport/XlsxSheet.swift

Why nothing caught it

This is the interesting half. parity-governance triggers on:

Tools/issue_ref.py
Tools/parity_*.py
Tools/parity_*.json
Tools/tests/test_parity_*.py
Tools/tests/test_rr_legacy_preservation_contract.py
.github/workflows/parity-governance.yml

The authority is derived from Packages/** and android/**, and neither is in that list. So a pull request can invalidate the authority without ever running the check that guards it, and the next pull request that happens to touch Tools/parity_* inherits a red board it had nothing to do with.

Verified rather than assumed: the check is absent from the check list on the Lift Log pull request head AND from the post-merge push on 4453a089b. It did not run and go green, it never ran.

The daily 17 4 * * * cron is the existing backstop, and its own comment says as much: "so authority staleness is found on a schedule instead of by whoever next edits Tools/". Tonight's cron would have found this. A contributor found it first.

The deadlock

Both documented refresh paths refuse, from a clean main checkout:

$ python3 Tools/parity_ledger.py --refresh-derived
FAIL derived refresh rejected; snapshots restored
ERROR: Tools/parity_twin_map.json: base authority cannot be reproduced with the
       current derivation; migration required

$ python3 Tools/parity_ledger.py --refresh-derived --repair-stale-base
FAIL derived refresh rejected; snapshots restored
ERROR: Tools/parity_twin_map.json: stale-base repair rejected because finding
       identities differ from the exact base

Both roll back cleanly, which is correct behaviour. --repair-stale-base was added in #2212 for the previous generation of this problem (#2211); it does not cover this one, because Lift Log's three new files changed finding identities relative to the base as well as the file count.

A third attempt pinning --base 3c8bfe659, the last commit where the authority was reproducible, is running as this is filed; I will add the result.

Why I stopped rather than forcing it

The only remaining lever is --bootstrap-map --write-baseline, which rewrites the authority wholesale. The tool's own help says new semantic debt is governed by exact, manually reviewed typed dispositions, so a wholesale rewrite discards reviewed judgement to fix a file count. That is a decision for you, not something to slip quietly into a pull request that claims to repair a file count.

Impact

Correction: the trigger exclusion is deliberate, and my suggestion was wrong

My first version of this issue proposed adding the scanned Packages/** and android/** roots to the workflow trigger. That was wrong, and I am leaving the record rather than quietly deleting it.

The workflow header already documents the decision, and it already documents this exact failure:

Product changes are what invalidates it, and product changes are exactly what both filters exclude, so main could carry stale authority indefinitely with nothing red until some later PR touched Tools/ and inherited the whole accumulated drift.

That happened four times, and three landed on an outside contributor's PR that had caused none of it. The schedule finds it within a day, on main, where re-deriving is a two-minute maintainer action.

Deliberately a schedule rather than an unfiltered push: this suite is pinned NOT to run on product source (see test_core_tools_filter_covers_every_governance_tool_path, which asserts both filters and forbids product globs), and a periodic check keeps that true. See the history in #1534.

test_core_tools_filter_covers_every_governance_tool_path pins the path list exactly, asserts every non-.github path begins with Tools/, and carries explicit assertions against 'Packages/**/*.swift' and 'android/**/*.kt'. The guard exists to stop exactly the change I suggested.

So today is the fifth occurrence of a known, accepted trade, not a new hole. The trigger is not the bug.

What is actually new

The accepted remedy is "a two-minute maintainer action" on main. That action no longer works. Both documented refresh paths refuse, as recorded above, and --repair-stale-base was already the answer to the previous generation of this (#2211 / #2212).

That is the part that needs attention: the schedule can find the drift, and then nothing can clear it without --bootstrap-map --write-baseline, which discards the manually reviewed typed dispositions the tool's own help says govern semantic debt.

Concretely, the gap is a refresh path that can absorb new scanned files whose findings are new identities rather than changed values. Lift Log added three files and therefore three sets of identities that the exact base has never seen; --repair-stale-base is scoped to metadata drift within an otherwise-matching base, so it declines.

Scope of this issue

Narrowed to that: make the documented recovery work. Widening the trigger is out of scope and, per the header, not wanted.

Activity

  1. changed the title [-]parity-governance: authority drifts on Packages/** changes that never trigger the check, and both refresh paths refuse[/-] [+]parity-governance: the documented authority refresh cannot absorb new scanned files, so a stale main has no recovery path[/+] on Sep 15, 2026
  2. ryanbr commented on Sep 15, 2026

    @ryanbr
    OwnerAuthor

    Third attempt changes the diagnosis. Pinning the base to the last commit where the authority reproduced gives a precise error instead of a vague refusal:

    $ python3 Tools/parity_ledger.py --refresh-derived --repair-stale-base --base 3c8bfe659
    
    ERROR: derived inventory changed without an exact issue-bound authority change:
      add-unpaired-function   swift  StrandImport/XlsxSheet.swift::parser/5#3
      add-unpaired-function   swift  StrandImport/XlsxSheet.swift::rawPart/2#1
      add-unpaired-function   swift  StrandImport/XlsxSheet.swift::sheetRelationshipIds/1#1
      add-unpaired-function   swift  StrandImport/XlsxSheet.swift::sheets/1#1
      add-unpaired-function   swift  StrandImport/XlsxSheet.swift::targets/1#1
      add-unpaired-function   swift  StrandImport/XlsxSheet.swift::worksheetPaths/1#1
      add-unpaired-propertie  swift  StrandImport/LiftProgramSheetImporter.swift::nilIfEmpty@property#1
      add-unpaired-propertie  swift  StrandImport/LiftProgramSheetImporter.swift::trimmed@property#1
    

    So this is not a deadlock

    There is a recovery path and it is behaving as designed. Eight new one-sided Swift declarations arrived with Lift Log, and the ratchet will not absorb new unpaired declarations silently: each wants an exact issue-bound typed disposition saying why it has no Kotlin twin. That is the "manually reviewed typed dispositions" the tool's help refers to, and refusing to invent them is the gate doing its job rather than failing.

    Worth noting the intent was already stated at review time. The Lift Log description called out XlsxSheet.rawPart as "internal test support for asserting the template's column lock, kept on purpose". The judgement existed; it was simply never recorded where the ratchet could see it.

    I withdraw "no recovery path" from the title. The accurate complaint is narrower, and it is about the error message.

    The actual defect

    The two documented commands give no indication that dispositions are what is missing:

    --refresh-derived                      base authority cannot be reproduced with the
                                           current derivation; migration required
    --refresh-derived --repair-stale-base  stale-base repair rejected because finding
                                           identities differ from the exact base
    

    "migration required" and "finding identities differ" both read as tool-level breakage. Neither names a file, a declaration, or the word disposition. Only --base <ref>, which is not an obvious thing to reach for and defaults to origin/main (precisely the ref that has drifted), reveals that the answer is eight reviewed entries.

    So: when the default base is itself the stale one, the refusal should still say what it would need. The information is clearly available, since pinning the base prints it.

    What I would suggest

    1. Make the default-base refusal print the same disposition list the pinned-base run prints. That alone turns a dead end into a task.
    2. Record the eight dispositions for Lift Log's one-sided declarations so main goes green. They are all StrandImport spreadsheet-reading internals with no Kotlin counterpart, consistent with what the Lift Log author already described.

    Happy to do (2) as a separate change if you want the board clear first, though the dispositions want your judgement rather than mine, since they are permanent statements about what is deliberately one-sided.

  3. changed the title [-]parity-governance: the documented authority refresh cannot absorb new scanned files, so a stale main has no recovery path[/-] [+]parity-governance: a stale default base hides that the authority refresh needs issue-bound dispositions[/+] on Sep 15, 2026
  4. ryanbr commented on Sep 15, 2026

    @ryanbr
    OwnerAuthor

    Full scope, and a correction: it is 54 requirements, not 8. My earlier list came from a truncated view and I built a plan on it.

    What the refusal actually asks for

    kind count dispositionable
    add-unpaired-function 26 yes
    add-unpaired-constant 24 yes
    add-unpaired-file 3 yes
    add-finding 1 no

    By file: LiftProgramSheetImporter.swift 24, XlsxSheet.swift 15, LiftMetrics.swift 12, LiftLogStore.swift 3.

    Why writing all 54 would be wrong

    Twelve of them are LiftMetrics constants: hardSetRpeThreshold, hypertrophyMinimumSetsPerWeek, oneRepMaxRepCeiling, strengthMinimumSetsPerWeek, strengthPlateauSetsPerWeek and friends. #2099 is explicit that this is pending work rather than a deliberate split:

    Not included: Android screens, and a Kotlin twin of LiftMetrics (pure functions; it can follow using the CLAUDE.md oracle method once the figures are settled).

    A platform_specific disposition is a permanent statement that a declaration is deliberately one-sided. Writing one for a twin the author has committed to would waive, forever, exactly the parity the ledger exists to protect. The alternative type does not fit either: experimental requires a unique issue per entry, so covering 54 would mean minting dozens of issues to satisfy a schema.

    There is also no way to express the last one. add-finding is not in allowed_kinds, so test-only-callsite|…XlsxSheet.swift::rawPart/2#1 cannot be dispositioned at all; a finding needs an accepted_findings entry in the compact baseline, with canonical reason and provenance drawn from BASELINE_REASONS.

    A practical trap in reading the refusal

    The identities are NUL-separated: the output that looks like add-unpaired-constant swift Packages/… is really add-unpaired-constant then swift\x00Packages/…. Two consequences for anyone debugging this:

    • redirecting the run to a file makes it binary, so plain grep silently matches nothing and reports no errors on a file full of them
    • any tooling that splits the line on whitespace to recover the identity will match nothing

    Both bit me. It is worth the refusal printing a whitespace-safe rendering, or a --json form, since the current output cannot be processed with ordinary text tools.

    Suggested shape

    1. Write the Kotlin LiftMetrics twin. That accounts for 12 of them legitimately, and it is the follow-through already described in Lift Log: an on-device gym log book, advanced by a double-tap on the strap #2099.
    2. Decide which spreadsheet internals in XlsxSheet / LiftProgramSheetImporter are genuinely Apple-only. Android has no program importer at all today: LiftingImporter.kt reads Hevy/Liftosaur session history into WorkoutRow and skips templates. The sheet template also accepts .csv, so an Android importer may never parse .xlsx.
    3. Handle the single add-finding through the baseline rather than the registry.

    That is design follow-through on Lift Log, not bookkeeping, which is why I have not written the entries myself.

    Still separate

    The property-kind defect that made one class of these impossible to declare is handled separately, in #2231. It stands on its own and is unaffected by any of the above.

  5. ryanbr commented on Sep 15, 2026

    @ryanbr
    OwnerAuthor

    Amending the count, and step 1 of the suggested shape is now up as #2232.

    The count moved, and the earlier figure was not wrong

    The 54 above was measured at 19:47. #2231 merged at 20:00. On current main
    the same refusal reads 62, and the difference is entirely #2231's doing:
    generalising requirement emission across all four kinds surfaced six more
    functions and the first two properties.

    kind pre-#2231 current main with #2232
    add-unpaired-function 26 32 27
    add-unpaired-constant 24 24 19
    add-unpaired-file 3 3 2
    add-unpaired-property 0 2 2
    add-finding 1 1 5
    total 54 62 55

    Worth stating because I nearly filed the opposite: my first reading of the new
    number was that 54 had been a bad capture, since the run it came from prints
    stale-base repair rejected as its first line. That line is the ordinary
    refusal banner, not a sign the totals were junk. The 54 was internally
    consistent and correct for the tree it was taken on. Checking the merge
    timestamp against the capture time is what separated the two explanations, and
    it is the check I should have run before reaching for the first one.

    What #2232 does to it

    Measured by running the ledger on a clean worktree at the same base and
    set-diffing requirement identities, rather than comparing totals between runs:

    • twelve retire: the six LiftMetrics functions, the five ReferenceDose
      constants, and the file entry, which follows from the function pairs
    • five arrive: fromRaw, Kotlin-only by design, plus four
      test-only-callsite findings

    Net 62 to 55.

    The five that arrive, stated plainly

    The four findings are accurate. Android has LiftEntities.kt and
    LiftingImporter.kt but no Lift Log screen, so nothing in the app calls the
    engine. volumeLoadKg and estimatedOneRepMaxKg avoided the same finding only
    because perExercise calls them internally, which is a property of the call
    graph rather than evidence they are wired up.

    So #2232 is step 1 of the shape above and it is honest about being an engine
    ported ahead of its consumer. It does not make the board green, and it is not
    meant to. Steps 2 and 3, the spreadsheet internals and the add-finding route
    through accepted_findings, are untouched and still want your call.

    One thing #2232 demonstrates about the pairing rule

    Twin claims are directional. Claims written only in the Kotlin files retired
    Kotlin-side entries and left all twelve Swift-side entries standing; the
    reciprocal claims in LiftMetrics.swift are what retired those. This is the
    same asymmetry CLAUDE.md notes for oracles, that one only guards the direction
    it is written in, and it is easy to miss when the twin plainly exists in both
    trees. Anyone working steps 2 and 3 will hit it.

  6. ryanbr commented on Sep 15, 2026

    @ryanbr
    OwnerAuthor

    Done in #2233, merged as 36b49dbed. parity-governance is green on main.

    What the remedy turned out to be

    --migrate-authority re-bases onto a freshly derived base authority when the
    base's stored manifest cannot be reproduced. It waives that reproducibility and
    nothing else: new one-sided declarations are still computed and still fail
    closed when undeclared, which is mutation-proven rather than asserted. The
    ordinary refusal now names the flag, and PARITY_GOVERNANCE.md carries it, since
    this issue asked for the DOCUMENTED recovery to work and the doc was pointing at
    --repair-stale-base, the one tool that cannot serve this case.

    Three things I recorded here that were wrong

    --bootstrap-map --write-baseline was never an available lever. It refuses
    outright when an authority already exists. I described weighing whether to use
    it; it was never on the table, which made the deadlock total rather than a
    choice.

    It could not have discarded the reviewed dispositions anyway. No refresh path
    writes parity_dispositions.json. PARITY_GOVERNANCE.md already said so, and I
    had read it.

    The 54, later 62, requirements were an artifact of my base. I measured
    against 3c8bfe6595, which predates Lift Log, so Lift Log's own declarations
    looked like new debt. Against origin/main, the base the tool defaults to, the
    delta is zero. That debt is already on main.

    That last one retires the follow-up I proposed here and repeatedly presented as
    needing a decision: deciding which spreadsheet internals in XlsxSheet and
    LiftProgramSheetImporter are genuinely Apple-only was never a prerequisite for
    a green board. It may still be worth doing as design follow-through on Lift Log,
    and if so it wants its own issue rather than this one.

    Also worth leaving in the record: the theory that a better base would work cost
    two full runs to disprove. 8039ad166 is the commit that last refreshed the twin
    map, and its authority still does not reproduce.

    What a refresh actually does, stated plainly

    It ADOPTS the one-sided declarations already on main into the authority,
    because the ratchet governs debt relative to base and merged debt is the base.
    Roughly 56 entered this time. That is the intended maintenance action, the same
    one as fe0f5c962 after the Oura merges, and it is why the daily schedule
    earns its place: it keeps the window to a day rather than letting drift
    accumulate until a contributor inherits it.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions