Skip to content

fix(desktop): ratchet renderer debt against the base tree, not its ledger - #4249

Merged
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:fix/renderer-architecture-ledger-session-collab
Aug 30, 2026
Merged

fix(desktop): ratchet renderer debt against the base tree, not its ledger#4249
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:fix/renderer-architecture-ledger-session-collab

Conversation

@liuxiaocs7

@liuxiaocs7liuxiaocs7 commented Aug 30, 2026

Copy link
Copy Markdown
Member

Summary

main CI has been red since #4088 on the test job → "Check renderer architecture" step, and stays red on every commit after it (e.g. #3741). The step runs apps/desktop/scripts/check-renderer-architecture.mjs --base <BASE_SHA>, which enforces two things: a snapshot (the committed renderer-architecture.json ledger must match the renderer source tree) and a monotonic-debt ratchet (--base: legacy renderer debt must not increase relative to the base commit).

#4088 introduced the ledger but generated it on a branch that predated the session-collaboration feature (#4196 / #4198) already merged into main, so the committed ledger under-reports its own tree — the snapshot check fails. It cannot be fixed by regenerating the ledger alone: the ratchet compares the committed ledger against the base commit's committed ledger, and since that base ledger is the stale one, recording the already-present debt looks like brand-new debt. Because CI's BASE_SHA always carries the stale ledger, no forward commit can record the missing entries — the ledger is permanently wedged.

This measures the ratchet floor against the base commit's actual source tree rather than trusting its ledger. loadBaseConfig materializes the base tree in a detached git worktree and re-derives its debt via generateArchitectureConfig, keeping the base ledger only for policy fields (hook transitions, growth directories, root-debt key set, ownership). The ledger is also regenerated to record the session-collaboration surface. This is the least-invasive fix that keeps the guard fully effective while removing the false positives a stale baseline produced.

Fixes#4250

Root cause

The ratchet was designed assuming the base ledger is consistent with the base tree. #4088's baseline violates that invariant, so validateMonotonicDebt (ledger-vs-ledger) reports phantom "debt increased" / "new unclassified file" for debt that already existed at the base commit. Deriving the base floor from the base tree makes the comparison tree-vs-tree, which is the property the ratchet actually wants.

Verification

  • node --test apps/desktop/scripts/check-renderer-architecture.test.mjs62/62 pass (the pure ratchet logic is unchanged; only base-config derivation changed).
  • node apps/desktop/scripts/check-renderer-architecture.mjs --base <main>passes.
  • Adversarial probe: added a renderer file absent from the base tree, regenerated the ledger, re-ran --base → still correctly rejected with new unclassified renderer source files are forbidden, confirming real regressions are still caught and the guard is not weakened.
  • biome lint / biome format on the changed script → clean.
  • In CI on this PR the "Check renderer architecture" step passes (the base-tree worktree works under fetch-depth: 0), and it is only materialized when --base is passed, so local check:architecture is unaffected.

Testing note: the existing 62-case suite covers the ratchet semantics (unchanged). The new worktree-based base derivation has no dedicated committed test; it is validated by the adversarial probe above and the passing CI step. A two-commit git-fixture integration test would be a reasonable follow-up.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Opus 4.8) — diagnosed the CI failure, designed and implemented the base-tree ratchet fix, regenerated the ledger, and ran the verification above. The affected commit carries a Generated-by trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 30, 2026
…dger
The renderer-architecture monotonic-debt ratchet compared the committed
ledger against the base commit's committed ledger. When a ledger under-
reports its own tree -- as apache#4088's baseline did, having been generated on a
branch that predated the session-collaboration files already merged into
main -- a faithful baseline correction looks like brand-new debt, and the
ratchet wedges the ledger permanently: no forward commit can record the
missing entries.
Derive the base debt from the base commit's actual source tree (via a
detached worktree) instead, keeping the base ledger only for policy fields.
Real regressions are still caught, since genuine new debt requires a source
change the base tree lacks. Also regenerate the ledger to record the
session-collaboration surface that was missing on main.
Generated-by: Claude Code (Opus 4.8)
@liuxiaocs7
liuxiaocs7force-pushed the fix/renderer-architecture-ledger-session-collab branch from 2093a3e to e78e070CompareAugust 30, 2026 08:30

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this at the existing renderer architecture authority.

I reviewed exact head e78e070d6. The failure is demonstrated on exact main: #4088 carried a ledger generated before the session-collaboration sources that were already in its final base, so the committed baseline under-reported the tree and made a faithful forward correction look like new debt. Re-deriving the comparison config from the base commit's actual detached worktree fixes that protocol gap while retaining the committed ledger as the authority for policy fields. It does not add a second checker or ledger.

I replayed the 62 checker fixtures and the real --base 8c491e64b35cd7af66437efc8020a88b171e2bb6 path; both passed. I also added a temporary base-absent renderer file, regenerated the ledger, and confirmed the ratchet still rejects it as new unclassified debt. Biome passed on both changed files. No P0-P3 findings.

The required CI job is still running; this approval covers the reviewed code and exact head, while merge remains gated on that check.

Review analysis was assisted by Codex. Astro-Han verified the exact head, root-cause history, base-tree authority path, focused checks, and adversarial regression probe, and owns this review.

@Astro-Han
Astro-Han merged commit 9ff2675 into apache:mainAug 30, 2026
1 check passed
liuxiaocs7 added a commit to liuxiaocs7/maka that referenced this pull request Aug 30, 2026
Three renderer files tracked by the architecture ledger drifted after
the ledger was last regenerated in apache#4249, each landing without a paired
ledger update:
- app-shell-chat-actions.ts nonTriviaTokens 4363 -> 4376 (apache#4246)
- app-shell-session-events.ts nonTriviaTokens 2931 -> 3042 (apache#4232)
- import-tasks-settings-page.tsx hookCalls useEffect 4->5, useRef 3->5 (apache#3905)
CI first went red on apache#4246 and accumulated the other two, wedging the
'Check renderer architecture' job on main and every branch cut from it.
Regenerate the snapshot (--write) to match the current source; no
runtime code changes.
Generated-by: Claude Code
saltand pushed a commit to saltand/maka-agent that referenced this pull request Aug 31, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI red on main: "Check renderer architecture" fails from a stale renderer-architecture ledger

2 participants

@liuxiaocs7@Astro-Han