Skip to content

chore(desktop): stop pricing sanctioned imports in the renderer ratchet - #4823

Merged
Astro-Han merged 1 commit into
apache:mainfrom
orangeCatDeveloper:chore/renderer-ratchet-sanctioned-imports
Sep 5, 2026
Merged

chore(desktop): stop pricing sanctioned imports in the renderer ratchet#4823
Astro-Han merged 1 commit into
apache:mainfrom
orangeCatDeveloper:chore/renderer-ratchet-sanctioned-imports

Conversation

@orangeCatDeveloper

Copy link
Copy Markdown
Contributor

Summary

The renderer architecture ratchet admits a legacy file's dependency on a validated copy catalog, a shell module, or a public application contract, but it still counted the importDeclarations / importSpecifiers of those edges as debt. A legacy file that replaced eight inlined lines with one import from application/contracts failed with importSpecifiers debt increased from 2 to 3, so the incentive ran backwards: copy the helper rather than import it. The same inconsistency hit copy catalogs: validateCopyCatalog permits bare package runtime imports, yet the closure ratchet priced @maka/core/ui-locale as new dependencyPaths debt, so a catalog could not share a code-to-copy lookup helper.

Import debt is now computed per source and excludes sources the dependency ratchet already sanctions for that section (catalogs for root entries; catalogs, shell, public application and public feature paths for AppShell files). A validated catalog's bare package runtime imports are exempt from closure debt, matching what catalog validation already allows. The ledger is regenerated so existing entries reflect the new counts; the check against origin/main passes.

Verification

$ node --test scripts/check-renderer-architecture.test.mjs
ℹ pass 101
ℹ fail 0

$ node scripts/check-renderer-architecture.mjs --base origin/main
Renderer architecture check passed against origin/main.

Before, on a legacy AppShell file importing one helper from application/contracts:

Renderer architecture check failed:
- src/renderer/app-shell-copy.ts: importSpecifiers debt increased from 2 to 3

After, the same edge passes; a private application/sessions import is still priced (both cases are fixture tests in this PR).

AI use

Select exactly one:

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

Tool(s) and scope: Claude Code drafted the ratchet change, its fixture tests, and this description; the rule and its boundaries were reviewed by hand.

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

@Astro-Han Astro-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.

Reviewed at 061f489. The inconsistency is real and the fix sits on the existing seam: isSanctionedDependencyTarget already decides what dependencyPaths ignores, and import debt now asks the same question. I checked the ledger regeneration (30 pairs, every one lower), the four validateDebtFile call sites, and that both fixture tests fail on the old code (importDeclarations changed; expected 0, received 2 for the sanctioned edge; dependencyPaths debt increased for the catalog import). Debt cannot hide: a new sanctioned edge still lands in dependencyPaths as a visible ledger diff, and pulling more symbols through an existing edge still moves nonTriviaTokens.

Three non-blocking notes, take or leave:

  • importDeclarations / importSpecifiers no longer mean what the key says (app-shell.tsx has 92 imports, the ledger says 81), while dependencyPaths still stores the raw set and only filters at compare time. Since this PR regenerates the whole ledger anyway, renaming to unsanctionedImport* is cheapest now.
  • An entry's expected value now depends on other files' zones: moving a module into shell/ will shift unrelated legacy entries and fail with importSpecifiers changed. That is the intended cost, but worth one sentence in the body so the next person knows to regenerate.
  • The catalog exemption is "any bare package", bounded only by validateCopyCatalog. Fine today; just naming the boundary.

Order: this is the shared base of #4457 / #4639 / #4641 (their copies of this diff carry a dead = () => false default that disappears on rebase), so it should land first. It conflicts textually with #4640 on one ledger entry; second one regenerates.

AI-assisted review: drafted with Maka; I verified the ledger deltas, the call sites and the old-behavior failures myself.

The ratchet already lets a legacy file depend on a validated copy catalog,
a shell module, or a public application contract, yet it still counted
the import declarations and specifiers of those edges as debt, so the
cheapest way to satisfy it was to inline the helper. A validated catalog
likewise may only carry bare package runtime imports, yet the closure
ratchet priced them, so catalogs could not share a lookup helper. Both
edge classes are now free wherever the dependency itself is admitted.

Generated-by: Claude Code
Claude-Session: https://claude.ai/code/session_01FXyhj5g8LK9st2JCUW477V
@Astro-Han
Astro-Han force-pushed the chore/renderer-ratchet-sanctioned-imports branch from 061f489 to 128b2e9 Compare September 5, 2026 06:47
@Astro-Han
Astro-Han merged commit 21702c8 into apache:main Sep 5, 2026
1 check passed
@orangeCatDeveloper
orangeCatDeveloper deleted the chore/renderer-ratchet-sanctioned-imports branch September 5, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants