Repository navigation
fix(stryker-js): plan shards the way the longest-processing-time rule says - #301
Merged
Merged
Conversation
… says The NoDependentMutantReproducesPureLptShards law failed deterministically on main at seed 328101905 and on a second counterexample it shrank to. Both were planner defects; the law's bound is right. - A command listing the same (project, mutant id) twice was grouped into one placement, while pure LPT places each listing on its own shard. PlanShardsCommand now refuses a repeated (project, id), and `plan` resolves --projects entries so one directory named twice is planned once. - Equal-cost groups were ordered by their placement key, which prefixes the project with NUL, so a project name holding a control character sorted differently from pure LPT. Ties now compare the project first, then the key. Each counterexample is pinned as a named example in the law it refuted, and a CLI scenario plans `proj-a,./proj-a,proj-b` and expects the same plan bytes as `proj-a,proj-b`.
…ut as a config error plan --projects compared directories by path.resolve while planProject labels each project by its real path, so a symlink alias (proj-a,link-a) slipped past the dedupe, reached the planner as one (project, id) listed twice, and PlanShardsCommand.make threw: exit 1 with a crash stack. The dedupe now keys on the same real path planProject uses. A command the planner still refuses (a duplicate, --target-seconds 0) now fails as PlanShardsRefused: exit 2 (ConfigError) naming the refused input and the flags to change. Engine.planRequest gains that error and Engine exports it, so the changeset is major.
systemfsoftware-maker
marked this pull request as ready for review
October 11, 2026 02:16
ryanleecode
approved these changes
Oct 12, 2026
ryanleecode
left a comment
ryanleecode
left a comment
Contributor
There was a problem hiding this comment.
Verified at 757066e: up to date with main ddeb67a; check, all 5 e2e legs, checker-parity and Nix green; 0 open threads; no skips, disables or caps. Report-only review findings P2 (realpath dedupe, typed PlanShardsRefused, symlink case) and P3 (changeset wording) applied. Only red: bench typescript-checker leg, which fails on every PR until #312.
systemfsoftware-maker
added a commit
that referenced
this pull request
Oct 12, 2026
…d fixture bakes, LPT planner Takes #303's per-project lockfile closure digest, #311's npm-ci fixture bakes from committed locks, #301's shard planner, and #293's lawful planMutantTests generators. - The incremental cache version moves to 7. Main and this branch each moved it from 5 to 6 for different layouts, so neither may read the other's file. runInputsDigestOf keeps #303's lockfile closure input, and the store key reaches it through the same function. #303's closure files, tests and oracle fixtures are byte-identical to main. - incremental-reuse.integration.test.ts keeps this branch's version: #303's only edit there was the version number in the incrementalSources scenario this branch deletes. - plan-request.schema.ts keeps #301's PlanShardsRefused and drops the two schemas this branch retired. - The planMutantTests law file takes #293's lawfulSubsumptionOf and drops this branch's untestedOf, which fixed the same draw. - The e2e closure install is per fixture, so only fixtures that request the S3 driver install it, and it carries #311's devDependency map. The harness stages each fixture's planned dependencies into its manifest, and the bake runs npm ci. fixtures:lock regenerated the locks: the verdict-store fixture gains its own lock, and the other seven gain the yaml dependency #303 added to the engine. - The docs say a store is shared only between checkouts at the same absolute path, because mutant ids hash the absolute file path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
plannow lays out shards the way its longest-processing-time (LPT) rule promises. Onmainthe law∀c_Mutants_≡NoDependentMutantReproducesPureLptShardsfails every time at seed 328101905, andplan --projects proj-a,./proj-a,proj-bexits 0 but writes 18 scheduled entries instead of 12, so proj-a's mutants are planned twice.What was wrong
An invalid input reached the planner, and a real CLI path produced it.
PlannedMutants with project''and id0000000000000000, costing4.1583512893691603e-308ms and0ms.PlanShardsCommandaccepted that command, which lists one (project, mutant id) twice. The planner then gave both listings one placement key (plan-shards.workflow.ts:176onmain) and stacked them on shard 1, leaving shard 2 empty. The real CLI builds exactly that command when--projectsnames one project twice. Two changes fix it together:PlanShardsCommandnow refuses a repeated (project, mutant id) where the command is decoded.plancollapses--projectsentries that are the same directory, compared by the real path the planner labels projects with. So a second path (./proj-a) or a symlink (link-a -> proj-a) no longer produces a duplicate.plan-shards.workflow.ts:195onmain). That key starts with a NUL, so project'\u0000'sorted differently from LPT's project-first order. Ties now compare the project first, then the key.A command the planner still refuses, such as
--target-seconds 0, now fails asPlanShardsRefused: exit 2 (ConfigError), naming the refused input and the flags to change, instead of a crash stack.Engine.planRequestgains that error andEngineexports it, so the changeset bumps major.Both counterexamples are pinned as named examples in the laws they refuted. No seed was pinned or skipped, and no assertion was loosened.
Verification
mainec9dcf6CONFORMANCE_REPLAY="property=271728433;seed=328101905;runs=100"plan-shardsproperty file, 10 fresh-seed runsproj-a,./proj-a,proj-b)samePlan: falseproj-a,link-a)plan --target-seconds 0stryker plan: the planner refused its input: Expected a value greater than 0 at ["targetSeconds"]. Pass --target-seconds above 0 …CI's full replay token for this seed (with
path=) still reportsPropertyRefuted … Shrunk input: undefinedhere. That comes from the replay harness: anypath=that does not resolve, evenpath=99999, is reported that way. This token's path was recorded againstmain's generator, which the new uniqueness check changes. The harness's own rerun line (seed only) passes.Known, pre-existing
These are present on
mainand out of scope here:audit --projectsdoes not dedupe its list, soaudit --projects proj-a,./proj-aprocesses the project twice.path=token that no longer resolves against the generator asPropertyRefuted … Shrunk input: undefined, rather than saying the path is stale.check's contract-version-law failure and the Bench legs are red onmaintoo.