mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
refactor/issue-2140-install-source-config
10 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
a79f0d2e81 |
feat: add symbol-aware depgraph authority overlay (#2157)
* feat: add depgraph authority overlay * fix: centralize depgraph authority declarations * refactor: make authority overlay declaration-driven |
||
|
Michał Pierzchała
|
e832325e87 |
refactor(substrate): split host mechanics into @agent-device/host-kit capability ports (#2088)
* refactor: split generic host mechanics into @agent-device/host-kit (#2082 W1) The shared src/utils closure that blocked the platform-family moves lands on declared owners: generic host mechanics form a new private @agent-device/host-kit package between kernel and capture-kit, and capture-kit keeps capture, snapshot, and recording behavior, depending on host-kit for the mechanics it needs. tar-stream and yauzl move with the archive code. Every seam's exported subpaths are pinned in package-boundaries.test.ts, the layering model ranks the new zone, R13's allow-list names it, and each seam carries an exact eager-closure row. ADR-0019's substrate amendment describes the layout. Tests that mocked two of the moved modules separately became duplicate same-seam vi.mock factories, where the second silently replaced the first; those are merged, and the mocks that production code reaches past are pinned at their injection points instead. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * refactor(host-kit): one narrow capability port per export The four technical barrels (exec/fs/values/request) grouped by category rather than by capability, so a consumer needing one mechanic evaluated unrelated ones. Each export is now a single capability over the host machine: command, process, diagnostics, retry, archive, file, request, version. A port re-exports only what a consumer of that capability uses, and every port carries its own eager-closure row. Most of the old values barrel was never host mechanics. Pure record readers, config-source values, result text, memoization, async scoping, coordinate validation, and device-scope parsing touch no process, file, or environment, so they join kernel's other primitives instead. Closures fall accordingly: capture-kit's png-worker-client from 20 to 10, png-resize from 28 to 18, session-teardown from 79 to 68, and the CLI from 386 to 380. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * chore: drop the migration inventories and trim the touched comments Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * docs: trim the touched host-kit and mutation-lane comments Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * docs: keep tool directives only in the touched files Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * docs: keep tool directives only across the touched tree Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * fix: point the Swift parity comment at the real TS twin and test The W1 move rewrote this citation to packages/contracts/src/mobile-snapshot-semantics.ts, which does not exist: the module went to capture-kit while isTapPointInsideViewport itself went to packages/contracts/src/snapshot-visibility.ts. The TS test line was left pointing at the pre-move path. Both now resolve. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH * fix: repoint comment citations at the homes this refactor moved them to The W1 move left ~20 comment citations pointing at src/utils/*.ts and src/request/*.ts paths that no longer exist. Each now names the capability port that owns the symbol, which survives further file moves: exec -> host-kit/command host-process, owner-identity -> host-kit/process diagnostics -> host-kit/diagnostics atomic-file, process-lock -> host-kit/file retry -> host-kit/retry request progress/cancel -> host-kit/request version -> host-kit/version ttl-memo, source-value, parsing, device-isolation, keyed-lock, success-text -> kernel subpaths Comment-only; no closure, budget, or behavior change. ADR citations are left as written, being dated records of the decision rather than live references. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018VngeKZH6zBuJzNBk5YzUH --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
aeb2ff8402 |
refactor(contracts): retire the platform/interaction compatibility façades (#2048)
#1959 granularized the contracts entry surface but left the wide platform/interaction façades in place as a compatibility surface for ~490 type-only importers. This mechanically moves every importer (~350 files) onto the granular subpath each symbol actually lives on, deletes the two façade files, and removes the fallow ignoreExports entries and eager-closure-budget rows that existed only to cover them. Eight previously-unexported source files needed new package.json subpaths (clipboard, keyboard, network-traffic, platform-plugin, platform-providers, runner-lease-context, screen-recording-runtime-host), and ./interaction now points directly at src/interaction.ts instead of the deleted barrel. .oxlintrc.json's no-restricted-imports rules for the two façades are removed since there's no wide facade left to warn against value-importing. |
||
|
Michał Pierzchała
|
4f8dc3f31e |
refactor: move selector engine into workspace package (#1589)
* refactor: move selector engine into workspace package
* refactor(selectors): trim the package façade to its real consumers
Follow-up to the selector-package cutover, from a structural review of it.
- Drop 15 façade symbols with no consumer anywhere in the repo:
selectorUsesKey (added by the cutover, never called), isNodeVisible /
isNodeEditable (the real helpers are contracts/snapshot's), normalizeText,
splitIsSelectorArgs, IS_PREDICATE_REQUIRED_MESSAGE, four nested Replay
types, SelectorDisambiguationDisclosure, and the four kernel type
re-exports every consumer already imports from kernel directly.
- Delete SelectorCapturePolicyInput.selectorExpression, which
deriveSelectorCapturePolicy never read; the policy varies only by
predicate, so it takes one now. Two of the four tests asserted that the
unread parameter had no effect and could not fail; they go with it.
- Return the Maestro export vocabulary to the maestro package. The cutover
inlined MAESTRO_TEXT/STATE_SELECTOR_KEYS' values into the CLI call site,
leaving both constants dead in the package that owns the concept and no
gate over the two copies. MAESTRO_SELECTOR_PROJECTION is now the one
statement of it.
- Dedupe SelectorDiagnostics and SelectorDisambiguationDisclosure, declared
character-for-character twice across the AST/string seam, and name the two
shared option shapes once instead of five inline copies. The parser-side
resolution types take an Ast prefix so the twins read as twins.
- Delete three identity wrappers: parsePrivateSelector,
selectorExpressionToMaestro, and the formatSelectorFailure forwarder —
nothing passes it a chain any more, so the SelectorChain | string union
and its branch go too.
- Delete internal/index.ts, an AST barrel whose only consumer was one test
in the same directory (renamed to engine.test.ts), and the match.ts
pass-through that existed to feed it.
- ReplaySelectorGrammar had three variants for two behaviors; 'wait' and
'ordinary' were the same path. It is 'is' | 'positional' now.
- Drop the deleted src/sdk/selectors.ts from .fallowrc.json's entry list.
Behavior unchanged. pnpm check green: 598 unit files / 5278 tests, smoke
35 passed / 3 live skipped, layering 71/71, depgraph 22/22, mutation config
45/45, fallow clean, package smoke sound. Counterfactual: pointing
MAESTRO_SELECTOR_PROJECTION.textKeys at the state keys turns three
replay-maestro-export cells red; restored before commit.
* test(selectors): split the engine aggregation test by source concept
`internal/index.test.ts` (renamed `engine.test.ts` when its barrel went away)
was a 708-line aggregation over the whole engine — past the 500-line tripwire
and mirroring no source module, so it also ran as one serial unit.
It becomes five files that each mirror what they test, plus the parser cells
folded into the existing parse test:
resolve.test.ts alternative fallback, strict uniqueness,
first-match existence
resolve-disambiguation.test.ts ADR 0012 ranking: deepest, smallest-area,
winner-vs-challenger disclosure, tie fallback
resolve-viewport.test.ts the visibility half: on-screen beats
off-screen, including inside an off-screen
scroll container
match.test.ts per-key matching semantics (text, role,
focused, appname/windowtitle, decoded
newline labels)
arguments.test.ts where the selector ends and the command's
positionals begin, both grammars
parse.test.ts +6 grammar/escape cells beside the existing
property tests
The login-form tree shared by resolve.test.ts and match.test.ts moves to
`__tests__/login-form-nodes.ts` rather than being copied into both.
All 27 cells are carried over unchanged and still pass; no file now exceeds
224 lines. pnpm check green: 602 unit files / 5278 tests, layering 71/71,
depgraph 22/22, mutation config 45/45, fallow clean over 127 changed files.
* revert(selectors): keep agent-device/selectors public, behind one AST subpath
The cutover removed the `agent-device/selectors` public subpath as part of
tightening the API. It is in use, so the removal is reverted: the subpath ships
the same ten symbols v0.20.5 shipped, with the same signatures.
That has to coexist with the reason the package façade is string-only, so the
AST leaves through one named door instead of the main one:
@agent-device/selectors string-in/string-out; every in-repo consumer
@agent-device/selectors/ast the published parser surface; one consumer,
src/sdk/selectors.ts
`packages/selectors/src/ast.ts` re-exports parseSelectorChain,
tryParseSelectorChain, isSelectorToken, the AST-taking findSelectorChainMatch
and resolveSelectorChain, isNodeVisible, isNodeEditable, and types
SelectorChain / SelectorDiagnostics. `formatSelectorFailure` keeps its
published `SelectorChain | string` first parameter as a shim here rather than
widening internal/resolve.ts back to a union — the compatibility obligation
sits at the boundary that owes it.
This is strictly narrower than main, where the AST was reachable from anywhere
in src/ via src/selectors/*. Two gates hold it there: facade-symbols.ts pins
./ast to exactly the v0.20.5 list, and package-boundaries.test.ts asserts
src/sdk/selectors.ts is the only file outside the package that imports it.
Restored alongside: the ./selectors export and tsdown entry/chunk group, the
.fallowrc.json entry, the package-exports supported-subpath list, and both
client-api.md sections. No CHANGELOG entry — nothing is removed any more.
pnpm check green: 602 unit files / 5278 tests, smoke 35 passed / 3 live
skipped, layering 71/71 (10 packages, 32 subpaths), depgraph 22/22, mutation
config 45/45, fallow clean over 129 changed files, package smoke imported all
12 published entry points with publint and attw passing. Verified functionally
against the built dist: the doc's parse -> findSelectorChainMatch example
returns the same shapes as before, resolveSelectorChain still returns an AST
`selector`, and formatSelectorFailure still accepts a chain.
* fix(selectors): correct the two expectations that still assume the removal
Review P1s on a792415a: restoring the public subpath left two gates asserting
it was gone.
- installed-package-metro.test.ts moved `agent-device/selectors` into the
blocked-specifier list. It goes back to the subpath smoke set, running the
same `isSelectorToken('||')` + `parseSelectorChain` check it ran before the
removal, so the file's only remaining delta from main is a formatter reflow.
- owner-files-no-leak.test.ts asserted `dist/src/sdk-selectors.js` was absent.
It requires the stable named chunk again, and still rejects an auto-numbered
`selectors2.js` fallback — the pair is what proves the restored tsdown chunk
group is doing its job, verified against a clean build.
PR body corrected: the removal is no longer described as intentional API
tightening.
* refactor(selectors): satisfy the widened fallow scope after rebase
main's #1591 (the follow-up filed from this review) removed `packages/**` from
.fallowrc.json's ignorePatterns, so the new package is audited for the first
time. Everything below is a finding fallow could not previously see.
Dead surface, all confirmed consumer-free:
- 12 type re-exports from the `.` façade whose shapes consumers only ever
reach structurally.
- MAESTRO_TEXT_SELECTOR_KEYS / MAESTRO_STATE_SELECTOR_KEYS, orphaned by this
branch's own MAESTRO_SELECTOR_PROJECTION change, and the test-util
SELECTOR_VALUE_HAZARDS. All three are module-local now.
- IS_PREDICATE_USAGE_HINT fails --production because its only consumer is the
is-argument-surface parity test. It gets a commented `ignoreExports` entry
rather than deletion: the constant is what makes the daemon and CLI raise
ONE hint instead of two copied strings (ADR 0010), so the test asserting
that is the point, not an accident.
`fast-check` is now declared by the package that imports it.
Duplication, split by what could be proven:
- `isUsefulVisibilityAnchor` existed character-for-character in both
packages/selectors and packages/maestro. Moved to
@agent-device/contracts/snapshot, which both already depend on and which
already owns this vocabulary. Safe because the `normalizeType` each copy
called is itself character-identical to the contracts one — checked before
moving, since a different normalizer would have silently changed which
nodes anchor.
- maestro additionally reimplemented `normalizeType`, `buildSnapshotNodeMap`
(as `buildSnapshotNodeByIndex`) and `findSnapshotAncestor`, all
character-identical to contracts'. Deleted in favour of the shared ones.
- The three scroll-ancestor walks are NOT deduped. They are structurally the
same walk but each uses a different scrollable predicate, and I have no
evidence the three agree; collapsing them would be a Maestro-conformance
change, not a cleanup. Both maestro sites now say so, and the work is filed
separately.
`projectSelectorExpression` (15 cyclomatic / 22 cognitive, written by the
cutover) splits into a dispatcher plus `readAgreedTextValue` and
`projectSelectorTerms`; all three are under threshold.
Rebase note: the one conflict, in package-boundaries.test.ts, resolved to
NEITHER side — #1591 had already deleted `AdReplayVerifiedTargetGuard` as an
unused export, and this branch deletes the seven ReplaySelectorPort names, so
the conflicting block is empty.
* build: record fast-check for packages/selectors in the lockfile
Declaring the dependency in packages/selectors/package.json without
regenerating pnpm-lock.yaml made every CI job fail in its install step with
ERR_PNPM_OUTDATED_LOCKFILE. My local `pnpm install --frozen-lockfile` printed
"+ 1 dependencies were added: fast-check@^4.9.0" and exited 0, which read as
success but was the same mismatch CI refuses.
Regenerated with the pinned pnpm 11.17.0, not the 11.5.3 on this machine:
11.5.3 rewrites peer-dependency resolution keys repo-wide (dropping
`(supports-color@7.2.0)` suffixes) and produced a 222-line diff. With the
pinned version the diff is the 4 lines this change actually needs, plus
pnpm's alphabetical re-sort of the root selectors entry.
|
||
|
Michał Pierzchała
|
0ee2a86129 |
refactor: extract contracts workspace package (#1499)
* refactor: extract contracts workspace package * fix: preserve screenshot diff result contract * test: stabilize Android keyboard smoke |
||
|
Michał Pierzchała
|
76453add71 |
refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) (#1494)
* refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) Extend the workspace with packages/* and move the kernel behind an enforced public API: packages/kernel with nine consumer-earned subpath exports (errors, device, snapshot, contracts, collections, rect, redaction, daemon-error, bounds — the last absorbed from utils as Rect vocabulary). Every kernel import repo-wide becomes the @agent-device/kernel/<sub> specifier; kernel tests move to src/__tests__/kernel/ and exercise the package surface. The root declares the package in devDependencies (workspace:*), tsdown bundles it (noExternal) so the published artifact and its runtime dependency manifest are unchanged. Gate rewiring in the same change, per the W0 brief: - R1 kernel-sink retires (physically subsumed); new R11 package-boundaries guards no-root-back-imports, relative tunnelling past exports maps, undeclared workspace deps, and non-exported subpaths, with runtime resolution pins via import.meta.resolve. - resolveImportEdges and mutation ownership follow workspace specifiers through exports maps, keeping R4 cycle checks, depgraph, and derived test ownership connected across the seam (kernel-errors still owns 495 tests). listSourceFiles includes packages/*/src. - kernel becomes an unranked zone; mutation registry, stryker mutate globs, and the mutation-affected workflow path filter move to packages/kernel/src/errors.ts. - check:affected gains packages/ ownership (manifests fail open); vitest and coverage include packages/*/src; fallow ignores packages/** (its resolver cannot follow workspace specifiers). - The affected-selector CI job installs dependencies: its closure now crosses workspace specifiers, and the R8 relative exception is unsafe for production src files (Node ESM does not realpath, so dual specifier/relative loads would instantiate modules twice). The R8 zero-dep set is pinned empty with that rationale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: address W0 review — mutation sandbox, exports-map resolution, tsc -b Review findings on #1494, all five: 1. contracts-schema-public.test.ts reads the kernel source at its packages/ path (fs access invisible to the codemod and typecheck). 2. Mutation lane: Stryker sandboxes the tree but pnpm's node_modules symlink resolves @agent-device/* back to the real repo, so mutants in the sandbox never load and vitest.related finds no tests. vitest.mutation.config.ts now aliases each EXPORTED specifier to its source (derived from exports maps, never a wildcard), keeping resolution inside the mutated tree. Validated: kernel-errors module runs end to end (dry run 3,984 tests, mutants killed, exit 0). 3. Layering/depgraph resolve workspace specifiers through the exports-derived map (workspaceSpecifierTargets) instead of reconstructing paths, so '.'-facade packages resolve; the positional fallback remains only for map-less fixtures (P0 pin). 4. Per-package project references implemented: packages/kernel is composite (emitDeclarationOnly -> dist-types, gitignored), the root references it, and typecheck becomes tsc -b — probed to catch type errors on both sides under TypeScript 7 native. 5. R11's relative-route exception now requires membership in an actual R8 zero-dep job closure (zeroDepClosureFiles walks entries), not mere scripts/ placement — closing the dual-instantiation bypass. Also from review discussion: daemon-error moves out of the kernel package to src/client/ — its consumers (cli, client facade) rehydrate wire DaemonErrors client-side; the daemon only produces them. Kernel drops to 8 exported subpaths before any of them ship. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * refactor: one exports-map reader for mutation alias and ownership Fallow flagged workspaceExportAliases (cognitive 15, CRAP 90). The manifest-reading logic already exists as workspaceSpecifierTargets in scripts/layering/package-boundaries.ts, so both the Stryker sandbox alias table and the mutation ownership walker now consume it instead of carrying near-clones. Behavior unchanged; mutation suite 45/45 and changed-code fallow green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: composite kernel without a root references edge FreeRange runs plain `tsc -p tsconfig.json`, and a root `references` entry makes non-build-mode TypeScript demand the referenced project's built declarations (TS6305) — a standing "build first" tax on every plain -p consumer (fr, editors). Keep the per-package composite project and build it in typecheck (`tsc -b packages/kernel` before the root and examples/sdk passes), but drop the root references edge: root consumption resolves through exports to source, identical to runtime and to the bundler. Probed: plain -p green with no prebuilt output; kernel-side type errors still caught by its own build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: R11 uses the layering parser; mutation config is a fallow entry Review blockers on #1494: - R11's private single-quote regex could miss a double-quoted or re-export route into packages/*/src. specifierSites now delegates to the layering model's parseImports (both quote styles, side-effect imports, re-exports, dynamic imports), with direct regressions for each formerly-invisible form. - vitest.mutation.config.ts becomes a declared fallow entry instead of a tolerated unused-file finding: the full-repo audit now reports it reachable (unused files 2 -> 1; the remainder predates this PR). FreeRange clean-checkout evidence: with packages/kernel/dist-types and every *.tsbuildinfo deleted, `pnpm check:freerange` reports 0 findings on this head — the TS6305 topology died with the root references edge in the previous commit; check:freerange has no build precondition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
adcbdda8f0 |
perf: speed up unit tests and streamline checks (#1488)
* perf: speed up unit tests and streamline checks * fix: validate canonical packaging workflows |
||
|
devin-ai-integration[bot]
|
e0b8463ef8 |
feat(scripts): blast-radius query — dependents, owning gates, live-coverage owners (#1425) (#1470)
* feat(scripts): blast-radius query over the depgraph model (#1425) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(scripts): keep --limit bound to its value in depgraph affected Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
devin-ai-integration[bot]
|
edca35d122 |
chore(deps): Renovate config, packageManager-derived pnpm in CI, repo-wide format (#1444)
* chore(deps): add Renovate config and enforce packageManager pnpm version in CI Refs #1422 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: bump pnpm to 11.17.0 and format the whole repo with oxfmt format/format:check drop their hand-maintained path list: oxfmt already skips node_modules and honors .gitignore, so the only exclusion list is .oxfmtrc.json ignorePatterns. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mutation): accept either quote style in the affected-lane path filter Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore(deps): keep fixture-app runtime deps as individual Renovate PRs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
f19864e486 |
feat(scripts): dependency-graph report over the layering gate's model (#1410)
* feat(scripts): dependency-graph report over the layering gate's model
Reports what the layering gate deliberately does not enforce, as JSON plus a short
summary. No renderer: the productive artifact is the JSON.
pnpm depgraph # -> .tmp/depgraph/graph.json + summary
pnpm depgraph:test
Dependency graph: 898 files, 4627 edges, 25 zones
value-import cycles (R4): 0
type-only/dynamic cycles (not gate-rejected): 8
spine back-edges (R5): 0
type-only spine inversions (R6): 42
transitively redundant value edges: 1338
The two numbers worth having are the ones CI cannot give you. Transitively
redundant value edges — where the target is still reachable at distance >= 2, so
the direct import changes nothing about what the module can see — need a real
reachability pass, not a grep. And cycle detection over type-only and dynamic
edges covers the loops R4 excludes by design. Both are candidate lists, never work
lists; at ~1300 the redundancy set is a place to look.
It reuses scripts/layering/model.ts, the same module check.ts uses in CI, so the
file set, zone partition, edge kinds and cycle definition are the enforced ones. A
second extractor would describe a graph nobody gates. Consequence worth having:
its R6 count reproduces TYPE_INVERSION_BASELINE, so a mismatch means one of the two
is stale.
This is the analysis half of a viewer that was built and dropped. The render cost
~2200 lines and needed a Fallow exemption for a 920-line canvas file, and nobody
read it. Everything here clears the repo's bar with NO exemption — scripts/depgraph
is deliberately absent from ignorePatterns, unlike scripts/layering, scripts/perf
and scripts/maestro-conformance.
Getting there meant fixing rather than suppressing: extracted `valueSuccessors`
(the value-edge adjacency was built identically in two places — a real clone),
split `buildGraph` into four named aggregation steps, split
`reachableBeyondDirectEdge` out of `markRedundantEdges`, extracted
`compareZoneEdges`/`crossedZonePair`, extracted `edgeKindCode`/`edgeFlags` from a
nested ternary scoring CRAP 42, and deleted `fileGroup` plus the `group` node field
once the cluster layout went.
Two additive exports on scripts/layering/model.ts: `zoneRank` and `targetDagZone`
(previously module-private). The gate's behaviour is unchanged.
`pnpm check` green, 4488 unit tests, 5 model tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* ci(layering): assert the depgraph report reproduces the gate's baseline
The report reads the same model as the gate, so its inversion count must equal
TYPE_INVERSION_BASELINE. That agreement was previously a nice property nobody
checked; the Layering Guard job now runs scripts/depgraph/model.test.ts, so the
two cannot be green independently. Verified by bumping a baseline entry by one and
confirming the job fails with a message naming the fix.
The count feeding the check is computed by `typeInversionsByPair`, which applies
the gate's rule — once per FILE pair, over the raw resolved edges — rather than
reading the collapsed edge list. That matters: `collapseEdges` keeps one edge per
pair with the strongest kind winning, and `dynamic` outranks `type`, so a module
imported both lazily and for its types would collapse to `dynamic` and drop out of
the count. No such pair exists today (measured: 0 of 42 inverting pairs), but a
number wired into a CI equality check must not be able to drift for a reason
unrelated to layering.
Stated honestly in the README and the test: this is a cross-check of the report's
extraction and the baseline against the real tree, not two independent algorithms.
The gate remains the authority — if they disagree, the baseline or the tree is
wrong, never the test.
TYPE_INVERSION_BASELINE is now exported for this purpose.
Not done here, deliberately: the ~1338 transitively redundant value edges are a
candidate for a loose growth-only ratchet later. They are a candidate list, not a
work list, and a hard count would be noise.
`pnpm check` green, 4488 unit tests, 6 depgraph model tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* fix(depgraph): make the source reviewable, and stop overclaiming removability
Four review findings, all of them real.
P1 - the implementation was binary. scripts/depgraph/model.ts contained two raw NUL
bytes used as map-key delimiters, so Git classified a ~346-line file as binary and
hid its entire diff behind `- -`. Replaced with a unicode escape: identical at
runtime, textual on disk. I had seen the symptom repeatedly - every grep on that
file printed "binary file matches" - and worked around it with python instead of
asking why, which is how it survived to review.
Guarded repo-wide rather than for this one file: a new test asserts no tracked .ts
under src/ or scripts/ contains a raw NUL, verified by reintroducing one and
watching it fail. Nothing else would catch a recurrence, and the failure mode is
silent - the code works, the review does not.
P1 - "transitively redundant" claimed removability it cannot support. Module
reachability does not carry bindings: if `a` imports `{ c }` while `b` only
re-exports it as `{ c as b }`, the path a -> b -> c exists and deleting a -> c still
breaks `a`. The fixture in model.test.ts is exactly that shape and its comment said
"removable". Reachability also says nothing about when a module's side effects run.
Renamed throughout to what it measures - `transitivelyReachable`,
`markTransitivelyReachableEdges`, and a summary line reading "value edges whose
target is also reachable at distance >= 2 (reachability only - not a removability
claim)". The caveats and the counterexample are now stated in the marker function,
the fixture comment and the README, and symbol-level analysis is named as what
deciding any individual edge would actually require.
P2 - build.ts had no coverage. Every test exercised model.ts, so the CLI could break
its output path, wire shape or summary silently. Added three subprocess tests:
default path plus summary-agrees-with-payload, `--out` honoured and valid JSON
written, and a trailing `--out` falling back rather than crashing (pinned so it is a
decision, not an accident). `pnpm depgraph:test` now runs inside `check:tooling`, so
`pnpm check` covers it.
P2 - README was wrong three ways: it queried `.tmp/depgraph/index.json` after the
output moved to `graph.json` (the documented command failed as written), it derived
inversions from collapsed `zoneEdges`, which can undercount, and it claimed both
that the report runs in CI and that nothing here runs in CI. The query now reads
`typeInversions` and was run verbatim; the CI sentence names exactly which single
test runs and states that nothing else gates a merge.
pnpm check green, 4488 unit tests, 10 depgraph tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
---------
Co-authored-by: Claude <noreply@anthropic.com>
|