Skip to content

Report a partly compiling source as a warning when recovery still stands - #2407

Closed
woksin wants to merge 3 commits into
feat/screenplay-phase1from
feat/screenplay-sp0024
Closed

Report a partly compiling source as a warning when recovery still stands#2407
woksin wants to merge 3 commits into
feat/screenplay-phase1from
feat/screenplay-sp0024

Conversation

@woksin

@woksin woksin commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Changed

woksin and others added 3 commits July 28, 2026 21:39
Any compilation error at all made SP0024 an error saying "nothing recovered
from it describes the application reliably". That claim is false whenever
recovery succeeded, and it usually did: a host handing over a compilation
assembled without the compile items a build generates leaves every reference
to a generated type unresolved, and the errors then sit in code declaring no
artifact while every command, event and reactor is read exactly as written.
One real application produced 607 such errors alongside a complete and
accurate document, which the host then discarded on the error alone.

The severity now follows how many artifacts were recovered from a declaration
no compilation error sits inside. None - nothing recovered at all, or every
declaration something came out of being one the compiler could not make sense
of - stays an error, because no part of that document can be trusted. Any at
all is a warning stating how many errors there were, how many artifacts came
through anyway and what usually causes it, so the result is successful and
the document is kept.

A count is used rather than a proportion because any threshold would make the
same recovery pass for a large application and fail for a small one. Zero is
the only number that means recovery was prevented rather than dented.

Suppressing "declares nothing" is unchanged and, in the warning branch, can
never apply - a warning is only reached when something was recovered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SP0024 suppressed the document check on the code alone, which was right while
it was always an error: a model recovered from symbols the compiler never
accepted describes an application that does not exist, so a document made from
it being poor is the broken build rather than a second defect.

Now that it can be a warning, that reasoning inverts. A warning says what was
recovered stands, and a document built from a model that stands is exactly
what the check exists for. Suppressing it there would hand back a document the
language rejects with nothing wrong reported and a successful result, which is
the one outcome the check was added to make impossible.

The suppression therefore follows the severity rather than the code.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generator takes a Compilation and nothing else, so assembling one the way
a real build would is the caller's job - and nothing said so. The part hosts
get wrong is source generators: no workspace loading mode runs them, and an
Arc application leans on generation heavily enough that a compilation loaded
without them is missing every type they emit.

Documents the contract, how to run the generators before handing the
compilation over, and what the generator does when a host does not - which is
now two different things depending on how much survived.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@woksin woksin added the patch label Jul 28, 2026
@woksin

woksin commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Reviewer context — deliberately kept out of the description.

This is agent-authored code that no human has read yet. Green CI is not sign-off. Do not merge without explicit review.

Stacked on #2406 — this targets feat/screenplay-phase1, not main, because it builds on that branch. It must merge after #2406; GitHub will retarget it automatically once that lands.

The rule. SP0024 is an Error exactly when the number of artifacts recovered from a declaration that no compilation error sits inside is zero; otherwise it is a Warning and IsSuccess stays true. That single count collapses both cases the issue names — nothing recovered at all, and everything recovered coming out of a declaration an error is written inside.

A threshold ("near-zero", or a proportion) was deliberately rejected: the same recovery would pass for a large application and fail for a small one, and no cut-off is defensible. Zero is the only count that means recovery was prevented rather than dented. "Inside a declaration" is measured against every DeclaringSyntaxReference, so an error in a build-generated partial counts as inside the artifact.

The suppression change is the part worth reviewing hardest. SP0024 previously suppressed SP0034 (the generator reading its own document back) unconditionally. That is now narrowed to the Error branch only. As an Error, SP0024 says nothing recovered can be trusted, so a poor document is the broken build rather than a second defect. As a Warning it says the opposite — the model stands — and suppressing SP0034 there would hand back a document the Screenplay compiler rejects while reporting IsSuccess == true, which is the exact outcome SP0034 was added to make impossible. Specified in both directions.

SP0024 → SP0001 suppression is unchanged and is now provably vacuous in the Warning branch: zero artifacts ⟺ zero slices, so a Warning is only reachable when something was recovered, and SP0001 could never fire there.

A live demonstration of the issue, found by accident. While verifying, the end-to-end gate reported SP0024 with 41 errors on TestApps/AspNetCore — because that project had not been built in the worktree, so the workspace resolved nothing. Pre-fix that state exits 1; post-fix it exits 0 with a Warning. CI is green because it restores and downloads the build artifact, but it is worth noting that the gate is one artifact-download change away from meeting this same condition for real.

Verification

  • Screenplay.csproj Release: 0 errors, 0 warnings. Screenplay.Specs and Screenplay.EndToEnd likewise.
  • 928 specs passing (884 before, +44). 18 were confirmed to fail against pre-fix code, spanning both branches and the boundary; the two Error-branch severity facts pass either way by construction, and only their message assertions fail.
  • Each commit verified standalone on a detached HEAD.
  • End-to-end gate: exit 0, "The generated document reads back clean".
  • The motivating case is proven hermetically from source strings: a slice that compiles plus a helper referencing a non-existent generated designer type yields 1 compilation error, 2 artifacts recovered, Warning, no error-severity diagnostic, and a document that compiles with IsSuccess == true.

One imprecision, left deliberately. The Warning's "N artifacts recovered" counts what the readers read. Where two queries in a slice collide irreconcilably, one is dropped and reported separately as SP0022, so the count can exceed the model's by that one. It is evidence in a message rather than a contract.

@woksin

woksin commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #2408, which now targets main directly and carries all 19 commits from #2406, this branch and feat/screenplay-phase2 as one change with combined release notes. Nothing is lost — every commit here is an ancestor of that branch, and the branch is kept.

The SP0024 calibration and its narrowing of the SP0034 suppression are both described in the consolidated reviewer comment on #2408.

@woksin woksin closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant