Fix the complex-form benchmark's build and realign its docs with what it runs - #677
Merged
Conversation
…entation Removing the legacy comparison engine (#643) deleted the benchmark harness's trailing summary block along with the legacy stage that preceded it. That block held the only `return` on the fall-through path of an `int`-returning top-level entry point, so `benchmarks/complex-form-doc` has not compiled since — CS0161, "not all code paths return a value". Nothing noticed for a week because the project sits outside `Docxodus.sln` and no workflow builds it. Restores the summary line and the `0`/`2` exit code, and adds a CI step that compiles every out-of-solution tool and benchmark so a library change cannot rot one silently again. All eight such projects build clean in Release today. The same removal left `README.md` and `FINDINGS.md` promising a `WmlComparer` stage, two expected `[check] legacy ... FAIL` lines, and an exit-code story that the harness could not produce. The README is rewritten against what the harness actually runs: its stage table is now keyed on the exact `Bench(...)` labels, the exit codes are spelled out (including that a throwing stage is caught and counted rather than propagated), and it points at the in-repo `TestFiles/NVCA-Model-COI.docx` fixture, which is committed despite the old text saying otherwise. `FINDINGS.md` is refreshed from a recorded run of this commit — exit 0, all eight checks passing — with the input digest, runtime, and command captured, and with stable assertions separated from one-run timings and revision counts. The legacy engine's measurements move to a clearly-marked historical section explaining why that engine was retired. `ComplexFormBenchmarkContractTests` ties the two together: it asserts the README and `Program.cs` name the same set of stages, in both directions, plus a vacuity guard so an empty match cannot pass silently. Against the pre-fix README it fails three of its four assertions. Closes #662 Closes #669
JSv4
pushed a commit
that referenced
this pull request
Sep 2, 2026
#676 retires the NoWarn list and fixes the bugs CS8073 was masking. Two of its ten sites are in WmlToHtmlConverter.cs, which is on this demo's conversion path — but its own commit message establishes both as provably no-op: the following `as ImagePart` / null check already subsumes the not-found case, so the dead check was redundant rather than wrong, and is deleted rather than repaired. The eight genuine behavior fixes are all in DocumentBuilder (merge/split) and MetricsGetter, which this demo never calls; confirmed by grep before deciding. So no local rebuild and no measurement — CI runs the full browser spec on this merge, and neither the changed converter lines nor the fixed paths can move what the demo does. #677 is the complex-form benchmark's exit contract, unrelated. Release-merge checklist re-run since #676 touched CHANGELOG: the REDLINE THEATER entry is still inside [Unreleased], not swept into a released section.
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
1 participant
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.
Why
Two related pieces of rot left behind when the legacy
WmlComparerengine was removed in #643.The harness does not compile. The removal deleted the benchmark's trailing summary block along with the legacy stage that came just before it. That block contained the only
returnon the fall-through path of anint-returning top-level program, sodotnet build benchmarks/complex-form-doc/ComplexFormBenchmark.csprojhas failed withCS0161onmainever since. Nobody noticed for a week because the project sits outsideDocxodus.slnand no workflow builds it.Its documentation describes a stage that no longer exists.
README.mdstill listedWmlComparer.Compareas a measured stage and told readers to expect two[check] legacy ... FAILlines as normal;FINDINGS.mdpresented the legacy engine's timings and invariant failures as a current result.What changed
The build (#662). Restored the summary line and
return FailedChecks == 0 ? 0 : 2;.The reason it rotted. Added a CI step that compiles all eight out-of-solution tools and benchmarks (
complex-form-doc,docxdiff-stress,delivery,diffharness, the threemanifest-fuzzprojects,screenshots). All build clean in Release today, so the step is green from the first run and red the next time one breaks.The docs (#669). The README is rewritten against what
Program.csactually does. Its stage table is now keyed on the exactBench(...)label strings rather than prose category names, the exit codes are stated explicitly — including the non-obvious one, that a stage which throws is caught and counted like a failed check rather than propagating — and it points atTestFiles/NVCA-Model-COI.docx, which is committed in this repo despite the old text saying the reference document was not.FINDINGS.mdis refreshed from a real run at this commit, recording the input digest, runtime, command and exit code, and separating the eight stable check assertions from the one-run timings and revision counts that shouldn't be read as thresholds. The legacy engine's numbers are kept, but under a clearly-marked historical heading that explains they are the record of why that engine was retired, not a description of current behaviour.The drift guard.
ComplexFormBenchmarkContractTestscompares the stage names in the README table against theBench(...)labels inProgram.csin both directions, and asserts the README no longer names the removed engine. A fourth test asserts both extraction regexes match something, so the comparison can't pass vacuously by comparing two empty sets.Validation
dotnet build benchmarks/complex-form-doc/ComplexFormBenchmark.csproj -c Release— succeeds (Debug and Release).0,ALL CHECKS PASSED, all eight checks green. That output is whatFINDINGS.mdnow records.SA1633, the file-header rule every file in the repo trips); the test-project baseline inCLAUDE.mdmoves 689 → 690 accordingly.Closes #662
Closes #669
🤖 Generated with Claude Code
https://claude.ai/code/session_01VSnwkK1Nx6zZb2RnnoxKdx