defaults guard: a shared helper name is no tunable source (6182 -> 449); an unchecked BLOCK still counts
Summary
Follow-up to #33111. This branch contains the two commits of #33111. After #33111 lands, the diff here is only the last two commits.
1. A shared name is not a call of a Tunable helper. toggle, make and points are Tunable helpers. The old guard selected any declaration with a token of that name. For example, registry.bind("toggleSidebar") { ...model.toggle() } in AppActions.bindApp was selected, and the dependency walk then pulled in every handler's static func bind. Thus almost every app PR "changed a default". #32983 (OnboardingHandlers.swift:bind) and #32979 (NewTabPage.swift:paletteActions through HistoryHandlers.bind's closure make) were flagged only because of this.
Now a helper call counts only in two forms:
Type.helper, whereTypeis declared in the helper's file- a bare
helper, in a file that declares one of the types of the helper's file (the same type or an extension of it)
x.helper and .helper are other members. The dependency walk is not changed.
Fingerprints on feat-cmux-next 38369c1a886: 6182 before, 449 after. No entry is added. All real helper callers stay, for example ChromeTunables.swift:space1, TabTunables.swift:compactTabWidth, MetricTunables.swift:sidebarWidth and every DropOverlayTunables entry. The bot-owned snapshot schemas/defaults/cmux-next.json is regenerated by base autofix; this PR does not edit it.
Results on the real diffs (defaults_guard.py check --base <merge base> --head <pr head>):
| PR | old guard | this guard |
|---|---|---|
| #32983 | tunable-source.CmuxNextApp/Handlers/OnboardingHandlers.swift:bind | no default change |
| #32979 | NewTabPage+Open.swift:fillPaletteActions, NewTabPage.swift:paletteActions | no default change |
2. Review findings from #33111
- If the write-access lookup for a receipt author fails, that author now counts for a BLOCK, never for a SHIP. Thus an unchecked receipt can only keep the guard red. Before, the receipt was dropped, so an unchecked BLOCK lost to an owner's earlier SHIP.
- If a lookup fails at the earlier head of a carried SHIP, the guard now names it in the error, as it does at the head. Before, it gave only a warning.
Regression command: python3 tests/test_defaults_guard.py
- Test commit 9094a6552df: 4 failures.
test_a_member_that_only_shares_a_helpers_name_is_not_a_tunable_sourcereproduces thebindApp/bind/makeshapes.test_an_unchecked_block_after_an_owners_ship_keeps_the_guard_redtest_an_unchecked_receipt_one_base_merge_back_is_named_in_the_errortest_a_receipt_whose_lookup_failed_is_named_in_the_error, for its new wording.
- Head: 72 tests OK.
- Real default changes are still caught:
- The existing fixtures (helper, helper body, cross-file
Factory.number, symbol, external symbol, dynamic, backticked) still fail without a review. - The new
test_a_helper_called_from_an_extension_or_by_its_type_is_still_a_tunable_sourcecovers an extension caller and aType.helpercaller in another module. - The name-collision test also checks that a real helper default change is still caught.
- The existing fixtures (helper, helper body, cross-file
Observed / Not run
Observed:
- The fingerprint counts before and after on the feat tree.
- Both PR diffs, run through the old and the new guard.
- The test red and green results above.
scripts/cmux-next/prepush-gate.pyis clean.
Not run: a live pull_request_target run. The guard runs from the PR base, so this change is first used by PRs whose base contains it.
Changelog
none
![Autofix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark-3x.png)
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.
Summary by cubic
Fixes the defaults guard matching any token that shares a Tunable helper's name (toggle, make, points) as a helper call, which flagged nearly every app PR as changing a default; a helper now counts only for calls that can actually reach it, including Self.helper, typealiases of Tunable, and subclass or protocol-inherited helpers. This drops the fingerprint count on feat-cmux-next from 6182 to 449, and #32983 and #32979 no longer report default changes. Also makes an unchecked receipt whose author's write-access lookup fails count for a BLOCK only, so it can keep the guard red but never green.
- A call owner's receipt is trusted without the write-access REST lookup, which was exhausting the shared API quota (#32983).
- Failed lookups are now named in the error, including at the earlier head of a carried SHIP, whose author's SHIP no longer counts but whose BLOCK does.
- Calls through a generic parameter (e.g.,
T.points) are still not matched.