Repository navigation
ci(graph): assert named entrypoints in graph preflight presets - #338
Conversation
Replace the directory-prefix `expectedFiles` check in tools/graph/run-graph-presets.mjs with a per-preset `namedEntrypoints` contract matched by file and symbol name against the graph result's answer surface. Tour presets seed the same names through `reinterpretations`, details presets derive the list from their handles. Add a drift-simulation test, document the rule in tools/graph/README.md, and link it from docs/TTSC_TOOLCHAIN.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue remains after the normal checks. Pre-merge checks |
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
… checks Address pre-merge review of #338: - Tour presets no longer write request.reinterpretations; normalizeRequest derives it from namedEntrypoints (an explicit field must equal the derived list), so the asserted list and the seeded list are one list. - Ad-hoc --request details files may use bare-name handles; they are skipped instead of throwing. Checked-in presets still must pin file#name:kind ids. - matchesNamedEntrypoint compares already-normalized fields; the on-disk declaration test accepts export lists but not re-exports or alias targets. - Note that test-file anchoring is not asserted (i18n preset description and README) and that the @ttsc/graph internal requires must be re-checked on version bumps. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rspress serves docs/ as the site root and treats ../tools/graph/README.md as a dead link; use the repository blob URL like the other out-of-site references. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Summary
pnpm graph:preflight(run in thebuild-web-targetsCI job) used to require that each ttsc-graph tour's anchors start with a directory prefix (expectedFiles). The tour ranking is a heuristic over the whole graph, so unrelated feature PRs shifted it: #264 folded apackages/game-servicesmodule back into an existing file to keeptools/target/anchored, and #337 added anexecuteNativeTargetBuildhint totarget-release.jsonfor the same reason.This PR replaces the directory-prefix check with a named-entrypoint contract:
namedEntrypoints: [{ name, file, kind? }], the symbols its question is about. For tour presets the runtime derivesrequest.reinterpretationsfrom that list (checked-in presets no longer write the field; an explicit field is accepted only if it equals the derived list), so the names the tour seeds and the names the preflight asserts are one list. Details presets derive the list (kind included) from their pinnedfile#name:kindhandles.tools/graph/local-graph-runtime.mjsgainsgraphResultSymbols/missingNamedEntrypoints/assertGraphPresetResponse: a named entrypoint is satisfied only by a graph node id in the result whose file and symbol name match (searched across tourentrypoints, flow starts, flowreachednodes, id-bearing anchors, and detailsnodes). Edge anchors such ascalls foo(no id, call-site file) and same-named symbols in other files do not count. Paths normalize\and leading./.tools/graph/run-graph-presets.mjskeeps thenext.action === 'answer'and non-zero anchor checks and reports the missing symbols and files by name.expectedFilesis removed (rejected with a migration message) rather than kept as a secondary check; the file header documents why.bootstrapStarter,createGameServicesBackend, andwithTargetAvailabilitydrop out while they were present. Deriving the seed list fromnamedEntrypointsremoves that failure mode by construction.pnpm graph:inspect --requestfiles stay lenient:namedEntrypointsis optional and bare-name details handles are skipped rather than rejected; only checked-in presets are asserted.i18n-locale-resolutionpreset previously listedpackages/i18n/test/i18n-smoke.tsinexpectedFiles); tours cite tests through their own test-coverage anchors, and the preset description and README say so.tools/graph/README.mdexplains presets, the rule, how to add a preset, and why feature PRs must not edit presets to make preflight pass;docs/TTSC_TOOLCHAIN.mdlinks to it.Rule
A preset passes when the graph answer is answer-ready, has anchors, and cites every
namedEntrypointsentry by file and symbol name. Feature PRs must not edit presets to make preflight pass; a failing preflight means a named entrypoint was renamed/moved/removed (update the preset in the same PR as the rename) or its name became ambiguous in the graph.Per-preset named entrypoints:
target-releaseexecuteNativeTargetBuild(tools/target/native-build-execution.ts),validatePlatformTargetsFile(tools/target/validate-platform-targets.ts),generateReleaseManifest(tools/target/generate-release-manifest.ts),verifyTargetArtifacts(tools/smoke/verify-target-artifacts.ts)bridge-contractsassertBridgeRequest,assertBridgeResponse(packages/bridge/src/index.ts),parseBridgeRequest(adapters/ait/src/host.ts)package-publishdiscoverBuildablePackages,sortByWorkspaceDependencies(tools/package/workspace.ts),selectBuildablePackages(tools/package/build-packages.ts),npmPackDryRun(tools/package/pack-packages.ts)i18n-locale-resolutionreadMpgdPreferredLocales,resolveTargetMpgdLocale,normalizeMpgdLocale(packages/i18n/src/index.ts)starter-loop(cwdexamples/phaser-starter)bootstrapStarter(src/bootstrap.ts),installStarterPlatform(src/platform/installStarterPlatform.ts),createStarterGame(src/runtime/createGame.ts),createStarterGameServices(src/platform/gameServices.ts)target-config-runtimewithTargetAvailability,createTargetRuntimeSnapshot(packages/target-config/src/runtime.ts),createEffectiveTargetConfigMatrix(packages/target-config/src/effective.ts),validateEffectiveTargetConfigMatrix(tools/target/effective-config.ts),generateEffectiveTargetConfigArtifacts(tools/target/generate-effective-target-config.ts)game-services-backend(worker tsconfig)createGameServicesRouter,createGameServicesBackend(packages/game-services/src/server.ts),createWorkerFetchHandler(apps/game-services-worker/src/handler.ts),createD1GameServicesStore(apps/game-services-worker/src/d1Store.ts)game-services-contract,tutorial-core,tutorial-driverTest plan
pnpm graph:preflighton the clean tree: all 10 presets pass.createDriftSimulationGameServicesBackendtopackages/game-services/src/server.ts, ranpnpm graph:preflight— all 10 presets still passed (exit 0), includinggame-services-backendandgame-services-contractwhose graph the new export changed — then reverted the simulation (not committed).pnpm test:tooling(40 tests, including the newtools/graph/run-graph-presets.test.mjs: synthetic tour whose ranked anchors all drifted intopackages/clipasses while the named entrypoints are cited; fails naminggenerateReleaseManifest (tools/target/generate-release-manifest.ts)when absent; edge anchors without ids and same-name symbols in other files do not satisfy; Windows separators /.//../; details handle derivation incl. kind; ad-hoc bare-name handles; derived vs. explicit reinterpretations; every checked-in preset's named entrypoints are declared on disk, with checked-in tours carrying noreinterpretationsfield).pnpm graph:preflight10/10 again on the derived-seed presets; captured-dump re-check 10/10; declaration-regex probe (13 cases:export type {},fromre-exports,asaliases, multi-line lists).pnpm docs:evidencepnpm format(no drift in untouched files);node --checkon the.mjsfiles.No changeset: CI tooling, docs, and private preset files only; no publishable package changes.
Local review
A reviewer subagent (sonnet) went over the working-tree diff for false passes, brittle matching, and Windows paths. Findings applied:
nodes[].calls/typesandambiguous[].candidates, so an unresolved handle could count as cited. The collector is now shape-aware: only the result's answer surface (tourentrypoints, flow starts/reached, id-bearing anchors; detailsnodes; trace start/reached) and only each entry's own id. Test added.name-field fallback loosened member matching (bug): a trace node's simplenamecould satisfymemberfor anyOwner.member. Matching is now the id's symbol part only. Test added with a simple-name trace node.kind: entries may pin a node kind; details-derived entries carry the handle's kind. Test added.normalizeGraphFilenow trims and runspath.posix.normalize(handles./,a/../b,//; keeps../../forcwd-scoped graphs). Both preset and result sides go through it.inspect-local-graph.mjs --requestregression: ad-hoc request files no longer neednamedEntrypoints(readRequestFileis lenient;readPresetstrict; the preflight refuses a preset that asserts nothing). Test added.includes; deadgraphResultFilesexport removed; README softened on how reliably tours cite seeded names and now states that checked-in tour presets list exactly their named entrypoints (enforced by the test).Not applied: capping tour presets at three named entrypoints (the five in
target-config-runtimeall land, confirmed on the clean tree and in the drift simulation; the README asks authors to verify each name withpnpm graph:preflight).Round 2 (pre-merge review, 6 items): (1) ad-hoc details files with bare-name handles no longer throw — fixed, test added; (2)
reinterpretationsduplication — fixed by deriving it fromnamedEntrypointsand dropping the field from checked-in presets; (3)matchesNamedEntrypointcompares normalized fields directly; (4) i18n test anchor — documented as not asserted (test variable names are neither stable nor seedable); (5) declaration regex widened toexport {}lists /function*, but re-exports and alias targets are excluded since the graph cites the declaring file; (6) comment added on the internal@ttsc/graphrequires. A second sonnet pre-push review of that delta found no bugs; its robustness items were applied (duplicate names seed once, ad-hoc explicit lists validated and trimmed, ad-hocnamedEntrypoints: []tolerated, clearer empty-handles message,assertGraphPresetResponsedocuments that it expectsnormalizeRequestoutput).🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Documentation