Skip to content

Import external events, ignore build output in slices, and read separators in names - #2406

Closed
woksin wants to merge 11 commits into
mainfrom
feat/screenplay-phase1
Closed

Import external events, ignore build output in slices, and read separators in names#2406
woksin wants to merge 11 commits into
mainfrom
feat/screenplay-phase1

Conversation

@woksin

@woksin woksin commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Added

Fixed

woksin and others added 11 commits July 28, 2026 20:40
…angling

A reactor observing what a sibling bounded context publishes refers to an
event a package declares. The document had nothing to say about it beyond
SP0021, and then referred to a name it never introduced - which the official
Screenplay compiler reports as an unknown event.

Screenplay already has the construct for this: the compiler reads the last
segment of an 'import' as the name of an event that is known. Every undeclared
name is now looked for in the assemblies the compilation references, and the
one that is found is imported rather than reported. SP0021 is left for what
is really unresolvable - a name nothing at all declares an event under, where
importing it would state that a package declares something it does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A compilation loaded from a project carries what every source generator
emitted to the intermediate folder. Those files declare real members of the
slice, so the folders they sit in were counted among the folders the slice
is written across - which reported a slice sitting in one folder as spread
over two, claimed the folder was shared with the next slice to be handed the
same generator output, and widened the screen scan to a build folder.

Generated source now answers neither where a slice is written nor where the
project is. Where a build wrote every last file the whole set is kept, since
answering with nothing writes every path against the machine that generated it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Kebab case is idiomatic for the runtime name of a Chronicle constraint and a
Screenplay identifier cannot carry a hyphen, so a transformation is forced.
Deleting the separators was the least readable one available - every one of
Ada's thirty one constraints came out as a run-together lower case word, and
the word boundaries the source stated were thrown away with them.

Each character an identifier cannot hold now ends a word instead, and the
words are joined in Pascal case: 'unique-timesheet-start' reads as
'UniqueTimesheetStart'. This is every identifier position rather than only a
constraint. A name carrying no separator is left exactly as it is, so nothing
the application already writes as one word changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolving a type strips the collection it sits in and then strips the
optionality of the element, while marking one as personal data stripped only
the collection. A collection of an optional value therefore registered the
mark under 'Nullable' - a name no concept is declared with - so the document
said the value was not sensitive while the runtime encrypted it.

Both now strip the same wrappers through one path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The statement pattern anchors on a line starting with 'import', which a
commented out one still does, so a component carrying the leftovers of a
change bound its screen to queries it never calls.

Comments are now removed before statements are read, and the line breaks a
block comment spanned are kept - joining the line after one to the line
before would hide the real import that follows it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only an interface parameter was read as a collaborator rather than as input,
which is not the whole of what a host hands a query. A cancellation token,
the page asked for and the order asked for are all filled in from the request
and none of them is an interface, so each was stated as caller input - a
parameter no caller sends, typed by a name the document never declares.

They are named explicitly, because there is no property of a type that tells
infrastructure from a value a caller really sends.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The sequence jumped from SP0028 to SP0030 with nothing saying why. A code is
what a consumer suppresses and groups on, so a number handed to something
else later would silently change what an existing suppression means. The gap
is now stated as deliberate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An accented letter can be written as one character or as a letter followed by
a combining mark, and Unicode calls the two canonically equal. A combining
mark is not a letter, so stripping first kept the accent of one spelling and
quietly removed it from the other - two documents for one application,
decided by how an editor happened to save the file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
C# says a value may be absent in two ways - a nullable annotation on a
reference type and a Nullable wrapper on a value type - and Screenplay says
it in one, a trailing question mark. Unwrapping distinguished the two all
along and nothing asserted that either survived as far as the printed text,
where getting it wrong describes a shape the application does not have or
names a type the document never declares.

The specification is end to end and covers a command property, an event
property, an enumeration, a collection of optional values and the parameter
of a query.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generator resolved them against the assembly it was reading and the
syntax builder resolved them again against the domain of the model, so one
generation ran two resolutions with two different fallbacks and let whichever
came last decide. They agree today, which is the only reason nothing showed.

The emitter is now the single place the emission half resolves, and the
builder is handed options that are already resolved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every Screenplay specification builds its compilation from source strings,
which is what makes them hermetic and what puts a class of defect out of
their reach by construction: a compilation built that way has no intermediate
folder, no source generator output on disk and no referenced package
declaring an event, so a generator that mis-attributes one of those produces
a wrong document that every specification still passes. Two shipped that way.

A console project reads a real project through MSBuild, generates from it and
reads the document back with the compiler the language ships. It has to come
back with nothing to say, warnings included - a warning there is the document
referring to something it never introduces, which is this generator being
wrong rather than the application.

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. Please treat the spec suite as a claim to be checked. Do not merge without explicit review.

The most important thing here is not in the release notes. Source/DotNET/Screenplay.EndToEnd/ is a new console harness, wired into dotnet-build.yml, that loads TestApps/AspNetCore through a real MSBuild workspace, generates a document, and compiles it with the Cratis.Screenplay compiler. It fails on any compiler diagnostic about the generated document, warnings included — a warning means the generator wrote a document referring to something it never introduces, which is a defect here rather than in the application. This is #2401 item 8, and it is the gate that catches the class of defect no specification built from source strings can reach: two of the nine defects found before this package first shipped were invisible to hermetic specs by construction, and #2394's dangling-event warning is a third.

A deliberate deviation on #2398. The issue also asks to prefer the declaring type name minus a Constraint suffix. That was not adopted. A single IConstraint may declare several rules in one Define, and naming them all after the type collapses them into duplicate constraint names within a slice; it also discards the runtime name, which is the thing the document is describing. On the real application it buys nothing — every constraint's runtime string already PascalCases to exactly the type name minus Constraint. Easy to add narrowly if a reviewer disagrees.

A limitation of the gate, stated plainly. The committed harness does not carry the resx PrepareResources workaround that the CLI has, so it would still trip over an application whose resource classes are generated during the build. TestApps/AspNetCore has none, so CI is unaffected — but if the gate should ever point at such an application, that fix needs an equivalent here. Verifying against the real production application required setting CustomAfterMicrosoftCommonTargets externally; no file in either repository was changed for it.

Verification

  • Screenplay.csproj Release: 0 errors, 0 warnings. Full Arc.slnx Release: 0 errors, 1 pre-existing CS0436 in TestApps/Chronicle that reproduces on main.
  • 884 specs passing (804 before, +80). Each spec was confirmed to fail against pre-fix code by reverting the change and re-running; the three items with no such spec (SP0029 reservation, the nullable/optional specs, the options refactor) are called out as guards rather than regressions by construction.
  • Determinism: two runs each over the real application and TestApps/AspNetCore, byte-identical.
  • Against the real production application, the generated document now reads back under the official compiler with 0 errors and 0 warnings (previously 1 warning on line 787).

Diagnostic counts on that application, same commit lineage before and after:

Code Before After
SP0021 1 0 #2394 — the event resolves and is imported
SP0025 3 0 #2395 — an obj/ generator folder no longer counts as slice source
all others unchanged unchanged
total 1065 1061

The whole document diff is the one import line plus its blank line, and 32 constraint names going from uniquetimesheetstart to UniqueTimesheetStart. Nothing else moved.

Note these differ from the figures circulating earlier (SP0015 ×4, SP0020 ×9, SP0024 ×1): those came from the published 20.65.0 through the CLI, not from current main. The before/after above are apples-to-apples from one code path.

Directory.Packages.props gained Microsoft.CodeAnalysis.Workspaces.MSBuild, Microsoft.Build.Locator and Microsoft.NET.StringTools for the harness. The harness sets TargetFrameworks (plural) to net10.0 deliberately — the singular form leaves the repository's Release multi-targeting in place and restore then hits an NU1109 downgrade on Microsoft.Extensions.Logging for net8/net9.

Comment on lines +97 to +104
foreach (var assembly in compilation.SourceModule.ReferencedAssemblySymbols
.OrderBy(_ => _.Identity.GetDisplayName(), StringComparer.Ordinal))
{
if (names.Any(assembly.TypeNames.Contains))
{
Collect(assembly.GlobalNamespace, names, found);
}
}
@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 this branch, #2407 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.

Closing this so there is one PR to review and one set of release notes rather than three that have to be merged in order.

@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