You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add find-repeated-build-work skill to dotnet-msbuild - #1311
Adds find-repeated-build-work to dotnet-msbuild for redundant target/task executions within one MSBuild invocation: invariant per-project work, unnecessary global-property project instances, per-item process launches, and racy run-once guards. It preserves required per-project/TFM work, output contents, and incremental builds.
Includes dedicated evals, fixtures, golden patches, and grader regressions. Existing-file changes are limited to reciprocal routing descriptions and the explicit coexistence skill catalog. Descriptions remain within the plugin's 15,000-character rendered-menu budget.
No agent changes, shared evaluation-tooling changes, or unrelated scheduling guidance are included.
Related issue
No linked issue.
Validation
Plugin conformance: passes for 21 skills and 3 agents.
python eng/eval-quality/check_eval_quality.py --base-ref origin/main: passes with existing warnings.
Pinned Vally 0.20 target/spec lint: passes with an existing scoring warning.
Real-SDK regressions cover native Git/SDK revision metadata, direct-project fallbacks and compiled attributes; equivalent schema rewrites; unchanged dependency timestamps, warm-cache BUILDINFO_STAMP changes, and submitted builds with unset/empty UTC defaults and bounded rollover retries; retired/inlined helpers; compiler history and the original writer race. All four golden rewrites pass.
Native MSBuild probes confirm that child-local values can suppress fallback, parent-local values need forwarding, and existing globals are inherited. The prior GPT loss on Trim inherited properties on an orchestrator task edge called inheritance "less defensive"; it was not an inheritance failure.
Current-head run 41086097250 on reviewed head b996f90 completed with failure. Both target comparisons are conclusive, with no errored or unmatched target trials:
Executor
Target result
Preference W/T/L
Dormancy: isolated / plugin
Positive activation: isolated / plugin
Claude Sonnet 5
VALID_PASS
5/18/0 (p=0.03125)
7/7 / 7/7
11/23 / 11/23
GPT 5.6 Luna
VALID_NO_CHANGE: activation contract failed
2/19/2 (p=0.6875)
4/7 / 7/7
18/23 / 10/23
GPT loaded the current description but still invoked the target for required-variant collisions, evaluation-only instances, and second-build reruns. Separate GPT sibling lanes also have unmatched trajectories and a six-minute trial timeout; those do not explain the target's genuine routing failures.
Previous production attempt: run 41051594296 on predecessor f97403d: GPT recorded 2W/19T/2L with the second-build dormancy guard still failing (6/7); Claude was inconclusive because one baseline trial timed out (dormancy 7/7). These are not successful current-head results.
Checklist
I searched existing issues and pull requests to avoid duplicates.
I kept this pull request focused and avoided unrelated refactors.
I added tests and evals for the new skill.
Existing plugin/test CODEOWNERS entries cover the added files.
No plugin metadata or external-domain changes require manifest updates.
Evaluation changes only
Scenarios are distinct and use natural prompts.
Golden rewrites and realistic mutation rejection are covered.
No-op, dormancy, and statistical power are covered.
Successful current-head production evaluation is recorded.
Previous eval failures were classified before content changes.
Separate current-head GPT/Claude evidence is available.
Adds a skill for finding work that MSBuild repeats inside a single build and
removing only what is safe to remove:
- the same Exec or task running in every project (for example a git lookup)
- a shared project built several times because callers pass different global
properties
- one process started per item instead of one for the whole list
- run-once guards (Exists checks, flag properties) that do not dedupe under -m
The skill separates these from legitimate per-project and per-target-framework
work, sizes the win against wall-clock before changing anything, and requires
the result to be validated with a new binlog.
Includes a Vally eval with six preference stimuli (four with golden patches,
a legitimate-counts audit, and a negligible-win stop condition) and one
dormancy stimulus, plus body-only cross-references from build-perf-diagnostics
and the two agents. Skill descriptions are kept short to fit the plugin skill
menu budget.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
👋 @YuliiaKovalova — this PR has 7 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the no-stale label to silence further pings.)
Require all component build roots, an actual build command, and an imported schema target connected to compilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This is presented as the skill's output contract, but none of the golden trajectories emits the table or Verdict: line and no grader checks either. The eval therefore cannot detect regression of this claimed skill delta, and its golden evidence models a contradictory format. Either update the goldens and add output graders, or make this format explicitly optional.
This accepts the empty-property guard on the whole target. In this fixture, that also skips the PropertyGroup that assigns InformationalVersion whenever build.sh supplies GitCommit, so a patch can pass while changing every stamped version. Require the Exec itself to be guarded, or explicitly prove that the version assignment remains on an unconditional execution path.
The reference check counts a ProjectReference even when it has a false Condition. Adding Condition="'false' == 'true'" together with the expected metadata passes this grader although the components no longer reference Core. Require at least one applicable Core reference per component before validating its metadata.
Textual ordering does not prove generation finishes before the build. For example, sh tools/make-buildinfo.sh obj/buildinfo.g.cs & appears before dotnet build and passes, but recreates the timing race this scenario is meant to eliminate. Exercise the script with an instrumented slow generator/build command, or otherwise reject background execution unless it is awaited.
A disabled generator still satisfies this check because task conditions are ignored. Adding Condition="'false' == 'true'" to the single Exec leaves its command and target hook intact, so every assertion passes even though no generated files are produced. Require the selected invocation to be unconditional or prove that its condition is true for the fixture.
The grader verifies the list writer but not that its parent directory exists. Deleting the original MakeDir still passes, yet a clean build fails when WriteLinesToFile writes $(IntermediateOutputPath)gen/schemas.list. Verify that a preceding step creates the list/output directory, or that the selected list path is in an already-existing directory.
This pattern accepts the opposite conclusion: output such as “the saving is not negligible, so optimize it” matches the bare word negligible. That lets the stop-condition scenario pass its deterministic output check while recommending the forbidden change. Require an affirmative leave-unchanged/not-worth conclusion rather than an unqualified keyword.
This is the only dormancy guard, but the new skill description defines two additional routing boundaries: repeated restores and slow evaluation. Without uncued expect_activation: false cases for those requests, the eval can pass even if the new skill hijacks work owned by build-perf-diagnostics or eval-performance. Add a separate recognition/restraint/redirection guard for each boundary.
Reference binlog-generation for capture, preserve the original build invocation, and make MCP analysis and the text-replay fallback explicit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The skill defines a findings table and Verdict: line as its output contract, but none of these graders assert that shape and every golden response omits it. A regression that ignores the contract therefore passes. Either enforce the table/verdict in the applicable output graders and golden trajectories, or explicitly make the section optional in the skill.
The prompt requires preserving the stamped informational version, but this only preserves its expression. Deleting or changing Directory.Build.props so Version is no longer 3.2.0 still passes while changing every assembly's informational version. Include the base version in the deterministic contract.
Verify generator execution and buildinfo output before building
This is only a substring check, so echo make-buildinfo or assigning the name to a variable before dotnet build passes even though the generator never runs. It also does not prove that the invocation writes the required obj/buildinfo.g.cs. Execute build.sh with mocked generator and dotnet commands, then assert one generator call with that output path occurs before the build call.
Compare invariant task counts with projects and required TFMs
The new pointer misses the skill's main signal: an invariant task repeated in every project has an N calls count equal to, not much larger than, the project count. It can also misclassify legitimate per-TFM work whose count exceeds the project count. Compare against the number of projects/TFMs that genuinely need the work and whether the command is invariant.
Distinguish invariant work from outer and inner project calls
This baseline contradicts the cases below: an invariant Exec repeated once per project is waste even when its count equals the project count, while a multi-targeted project itself normally appears once for the outer build plus once per TFM. As written, Step 1 can miss the primary git-stamp case and falsely flag the legitimate 4-calls-for-3-TFMs case. Distinguish project-specific task counts, outer/inner project calls, and build-invariant work here.
Avoid dividing sequential savings across unrelated busy nodes
Dividing aggregate avoidable time by all busy nodes is not valid for the per-item case this skill handles: 120 batched Exec calls inside one target are sequential on one node, so their wall-clock saving can approach the full aggregate time. Only divide when the duplicate calls actually overlap across nodes; otherwise this rule can incorrectly trigger the 1% stop condition.
Preserve executable version stamping and active references, exercise awaited generation, enforce schema preparation and output contracts, add restore/evaluation routing guards, and correct call-count and timing guidance.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The skill declares idle-node/serial-chain analysis as a real routing boundary, but the eval only has dormancy guards for incremental builds, restore, and evaluation. Add an uncued expect_activation: false parallelism stimulus that verifies recognition of scheduling/critical-path evidence, restraint from deduplicating legitimate work, and redirection to parallelism analysis; otherwise the overlapping -m/multi-project vocabulary can hijack those requests without being detected.
Add build-parallelism boundaries to router-visible descriptions
The router-visible description omits the explicit build-parallelism boundary present in the body. Requests involving -m, idle nodes, or an MSBuild task over a Projects list can therefore match both this skill and build-parallelism (whose description claims that task shape). Repository guidance at CONTRIBUTING.md:584-587 requires the boundary and reciprocal exclusion in both sibling descriptions; add those clauses so routing does not depend on body text that is only read after activation.
Accept equivalent revision property names and MSBuild stamp expressions through SDK behavior instead of XML spellings. Read actual version and title attributes from compiled assemblies with a dedicated reusable grader task. Observe Csc outputs to enforce the shared-file stimulus unchanged-rerun contract. Add focused acceptance and rejection regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The grader incorrectly rejects valid fixes through hard-coded item-name assertions and direct solution-invocation requirements, with acceptance regressions needed for equivalent rewrites and orchestrator-based builds.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The validation rejects valid rewrites because it incompletely expands output paths and generator commands and disallows helper files or imported targets.
Allow imported helpers while preserving original schema and evidence files. Observe expanded generator commands and effective output paths through the real build, with acceptance and preservation regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The grader accepts extra schema inputs and treats their generated files as expected, violating the unchanged-output requirement; enforce the exact original schema set and add an added-schema regression test.
Require the exact original schema input set so unexpected schemas and generated outputs cannot become expected results. Add a mutation regression while retaining imported-helper and golden acceptance.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because the tests reject valid cached-generation behavior and rely on an unsupported explanation for the reported file-busy error.
Permit current shared output to skip generation while preserving stale-input refresh, ordering, exact bytes, compiler inputs, and unchanged-build incrementality. Replace unsupported file-busy evidence with a reproducible interleaved-write compiler failure and focused regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because the Git mocks reject equivalent single-invocation revision lookups, making acceptance depend on command spelling rather than lookup semantics.
Forward instrumented Git calls to real temporary repositories and check lookup counts rather than exact arguments. Preserve revision and assembly checks, reject branch-name queries, and account for native SDK revision suffixes in early-hook regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because the evaluation can reject valid fallback or relocated-generator implementations while missing repeated compilation on unchanged builds.
Address the observed isolated GPT activation-contract violation by explicitly excluding second unchanged builds even when the incremental-build sibling is unavailable. Preserve the existing positive routing, skill body, evaluation scenarios, and budgets.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instrument the submitted generator in place without replacing its executable file or changing its script name. Accumulate native compiler observations so a later no-op build cannot hide unwanted unchanged-build compilation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The evaluator rejects valid rewrites that retire tools/make-buildinfo.sh because both generation detection and an earlier file read depend on that helper path.
❌ Evaluation did not complete successfully (the evaluate job reported failure). Check the workflow run logs, then comment /evaluate f97403dcfc8f92d57a0bc871745500a4947cf0d7 to retry this exact commit.
18 partial result file(s) were preserved for diagnosis but were not consolidated because the full matrix did not complete.
⚠️ Session replay telemetry was not published because the auxiliary dotnet/skills-data publisher failed. The evaluation verdicts above remain authoritative; maintainers must repair SKILLS_DATA_TOKEN or the publisher before replay links are available.
Separate Git metadata probes from revision lookups using native results. Validate shared-source bytes, timestamps and compiler behavior independently of generator paths, including retired helpers and fully inlined wrappers or native MSBuild tasks. Preserve a location-independent partial-write stress window and reject the original race and per-project rewrites.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because the grader’s assumptions about Git process counts, clock sources, and dependency timestamps can reject valid implementations.
Bound external revision queries without requiring a subprocess. Compare generated values with real UTC, leave unchanged dependency timestamps intact, and exercise a coherent prior-day cache. Add native regressions and lead routing with the observed second-build exclusion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The tests can reject correct builds during a UTC date rollover, and the rubric can penalize a supported SDK-metadata solution by requiring a specific Exec implementation.
Use one UTC day for the whole native scenario and restart once from a fresh workspace if it changes during or between builds. Keep normal failure and no-op checks intact, report repeated clock instability explicitly, and judge revision stamps without mandating an Exec implementation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the approved BUILDINFO_STAMP fixture override while retaining UTC by default. Advance that declared input in one native workspace instead of resetting cache state or changing timestamps. Cover stamp-aware and day-blind caches, native writers, default behavior, and wall-clock independence without constraining the generator entry point.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Every build probe sets a nonempty BUILDINFO_STAMP, leaving the required UTC default for unset or empty overrides untested and allowing regressions to pass.
Exercise unset and empty overrides through submitted builds with prior-day warm caches, unchanged reruns, and bounded UTC rollover retries. Reject missing or incorrect defaults while preserving native and helper-free implementations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allow child-local property sources to satisfy the fallback. Explain that missing parent-local values trigger child fallbacks unless forwarded, while existing global properties are inherited. Native MSBuild probes confirmed all three cases; routing and evaluation contracts are unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
❌ Evaluation did not complete successfully (the evaluate job reported failure). Check the workflow run logs, then comment /evaluate b996f901cde6f8a5da892d893b85e2c7c0f8888a to retry this exact commit.
18 partial result file(s) were preserved for diagnosis but were not consolidated because the full matrix did not complete.
⚠️ Session replay telemetry was not published because the auxiliary dotnet/skills-data publisher failed. The evaluation verdicts above remain authoritative; maintainers must repair SKILLS_DATA_TOKEN or the publisher before replay links are available.
This branch has not been deployed
No deployments
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
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
Adds
find-repeated-build-worktodotnet-msbuildfor redundant target/task executions within one MSBuild invocation: invariant per-project work, unnecessary global-property project instances, per-item process launches, and racy run-once guards. It preserves required per-project/TFM work, output contents, and incremental builds.Includes dedicated evals, fixtures, golden patches, and grader regressions. Existing-file changes are limited to reciprocal routing descriptions and the explicit coexistence skill catalog. Descriptions remain within the plugin's 15,000-character rendered-menu budget.
No agent changes, shared evaluation-tooling changes, or unrelated scheduling guidance are included.
Related issue
No linked issue.
Validation
python eng/eval-quality/check_eval_quality.py --base-ref origin/main: passes with existing warnings.BUILDINFO_STAMPchanges, and submitted builds with unset/empty UTC defaults and bounded rollover retries; retired/inlined helpers; compiler history and the original writer race. All four golden rewrites pass.Trim inherited properties on an orchestrator task edgecalled inheritance "less defensive"; it was not an inheritance failure.b996f90completed with failure. Both target comparisons are conclusive, with no errored or unmatched target trials:VALID_PASSVALID_NO_CHANGE: activation contract failedf97403d: GPT recorded 2W/19T/2L with the second-build dormancy guard still failing (6/7); Claude was inconclusive because one baseline trial timed out (dormancy 7/7). These are not successful current-head results.Checklist
Evaluation changes only