feat: adr system - #33
Merged
Merged
Conversation
Trax.Core has one decision worth recording that is genuinely its own: a mis-composed chain does not compile. The analyzer ships inside the package at analyzers/dotnet/cs and raises CHAIN001 and CHAIN002 at Error severity, so a junction asking for a type nothing produced stops the build rather than throwing somewhere behind a scheduler on the one code path that runs it. Writing the exemplar turned up a gap. The existing analyzer tests assert that a given source produces a given diagnostic, so they would catch a severity downgrade on a rule they already cover, but nothing covered the set itself: a third diagnostic added quietly, or an id renamed, changes what every consumer sees in their build and fails none of them. The ids are a published contract because consumers suppress and configure by id. Added a test that pins both the set and the severities, which also gives the ADR citation somewhere to live (that project asserts through CodeAnalysis.Testing and had no message channel, so it gains FluentAssertions and its lockfile is regenerated for it). Nine Meta guards here and all nine are shared, which is expected: the conventions Trax.Core would enforce are workspace-wide, and several of the engines behind them ship from here for consumers to subclass.
0001 said the analyzer reads the Chain() calls in a train's constructor. It triggers on Resolve() and walks the fluent chain back from there, requiring it to start with Activate(), and it bails out silently on any symbol it cannot resolve. Close enough to be believed and wrong enough to send someone looking in the wrong place. The new test's docstring claimed a renamed diagnostic id would fail none of the existing tests. It would: they expect a specific id at a specific severity, so a rename or a downgrade on a covered rule fails them. The real gap is narrower and is what the test actually closes, which is a third diagnostic added quietly.
Every guard class in this repo's Tests.Meta is now credited to a decision or explicitly opted out, and the adr-guard job checks it. Most of the shared guards enforce decisions in the central corpus, which is not present in this repo, so nothing local could name them. They cite the central ADR in their own docstrings instead: the guard treats a self-citation as classified, which is what makes the census usable across the split. Four opt out rather than cite. Extension-method naming and the test folder layout pin conventions that were adopted rather than chosen, the builder partial split is a documented pattern rather than a decision taken against alternatives, and the DI composition test is a smoke test that proves the wiring resolves and pins no choice. Each says so in its docstring, and the guard rejects a reason that reads as a deferral. Roughly half these classes had no docstring at all, so this is the first thing most of them say about themselves.
Written when it was still pending. Corrected in the same pass that switched it on, rather than left to go stale like the counts an audit caught earlier.
StripCommentsAndStrings ran five regexes in sequence, line comments before string literals. Any line holding a URL lost everything after the "//", including the closing quote, and the dangling quote then swallowed the file up to the next quote. One `"ws://localhost/trax/graphql"` collapsed a 375-line file to 44. Across the workspace that hid 10,677 lines in 148 of 928 test files from every source-scanning guard, and made every reported line number after the first URL wrong. Nothing failed. The guards reported green over source they could not see, which is the failure mode with no symptom. Comments and string literals are mutually exclusive contexts and cannot be resolved independently, so this is one left-to-right pass instead: line and block comments, raw strings of any quote count, verbatim and interpolated-verbatim with their doubled-quote escape, regular strings and char literals with backslash escapes. Content is blanked in place, one character for one character, newlines kept, so every offset and line number still points where it did. The shipped Trax.Core.Testing copy and the eight Tests.Meta copies all carried it; all nine are corrected together.
A reviewer traced every checkable claim in the corpus to source. These are the ones the code contradicted, in this repo. Where an ADR asserted an absence, the absence was usually smaller than stated: "not covered" is the more misleading direction to get wrong, because it invites someone to go close a hole that is already closed. Counts were recounted rather than carried forward, and the AGENTS.md central-ADR figure was derived from the index instead of written by hand, which is how it went stale twice.
Two repos pinned one commit of Trax.Docs and six pinned another, and neither included the census fix the six depend on. All eight now name the same one. It still has to be repinned once Trax.Docs merges. If that branch is squash merged the SHA is garbage collected and every repo breaks at once, so repin before merging any of these.
Twenty-three documents all closing with the same section, and six hand-tuned
variations on its header ("and the gap is wider", "and this is most of the
decision"). The editor's point was that the variation is the tell: a person
repeats a template without noticing, and decorating around it is what betrays
that someone noticed.
The changelogs carried sixteen same-day Corrected/Recorded pairs. The format spec
says that section records substantive changes, and a same-day revision of a
document nobody has read is a draft edit. Git holds those; the corrections are
described in the commits that made them.
Also drops 'genuine' and 'genuinely', which were filler in every one of the
fifteen files that used them.
The guard scanner routed $$""" through the verbatim-string branch, which mistook its $$" prefix for $@". A raw interpolated string containing a lone quote therefore ended the scan early, and everything after it was walked as code until a stray """ opened a literal that never closed, blanking every declaration below. Trax.Core.Testing ships this scanner, so the hole was in every repo that consumes it. All nine copies in the workspace now carry the same algorithm and the same remarks. The analyzer ADR claimed the analyzer cannot false-positive because it stays silent on anything it cannot resolve. It does not: a junction it cannot resolve is skipped and the walk continues against memory missing that junction's output. The ADR now names the case that actually bites, a junction named by a generic method's own type parameter, and says why a junction with no IJunction is a true positive rather than a false one. SourceTextTests carried one figure attached to the wrong defect. 93 of 928 test files lost code to the old stripper and two of those hid a real violation; 148 lost newlines, which moved reported line numbers without hiding anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LbjdoFEE8G2fzb2kuxxqoC
The previous pin pointed at a commit that no longer exists: re-signing the Trax.Docs branch rewrote every SHA on it. This points at the re-signed tip, which is pushed and resolves. It is still the pre-merge guard, so it wants repointing once more at the resulting commit on Trax.Docs main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LbjdoFEE8G2fzb2kuxxqoC
Nothing stopped NUnit arriving in an ordinary library, where it ships to every consumer as dead weight. NoTestFrameworkInSrcTests reads every src/*.csproj and fails on a test-framework reference outside an allowlist. The allowlist is the interesting part. Four Trax.*.Testing packages reference NUnit deliberately, because shipping architecture-guard fixtures is their product: a consumer subclasses the fixture and inherits its [Test] methods, so the attributes have to be in the shipped assembly. A second test fails when an allowlisted project stops referencing a test framework, so the exemption cannot outlive the thing it exempts. That check earned its place immediately by rejecting two *.Testing packages that ship helpers rather than fixtures. Repos with no src/ directory skip at run time rather than passing vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LbjdoFEE8G2fzb2kuxxqoC
Implements Trax.Docs/adr/0012. Each class an ADR names as an exemplar now carries
[Property("adr", "<path>/NNNN-slug.md")], and the guard resolves the claim through that rather
than by matching a bare class name against every class under tests/. The value mirrors the
Enforces docstring the class already had, so the two cannot drift.
The docstring citation stays as prose and the failure message stays as the thing a reader sees
when the guard goes red; the attribute replaces only the name matching.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LbjdoFEE8G2fzb2kuxxqoC
Trax.Docs merged, and it was rebase-merged, so every commit on that branch was replayed with a new SHA and the branch was deleted. The previous pin pointed at the branch tip and now resolves to nothing, which would fail this job with "action not found" on the next pull request. Repointed at the resulting commit on Trax.Docs main. The repin note is gone with it: the pin now tracks a commit on a branch that is not going anywhere, so the remaining advice is to read what changed before bumping it, since a newer guard can fail a repo that was green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LbjdoFEE8G2fzb2kuxxqoC
Trax.Docs was rebase-merged, which replayed every commit with a new SHA and signed none of them. Re-signing rewrote main again, so the SHA this job pinned a commit ago no longer exists and the action would fail to resolve. Repointed at the re-signed commit. Squash-merging avoids both rewrites: GitHub signs the one commit it creates, and the SHA it produces is stable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LbjdoFEE8G2fzb2kuxxqoC
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
This PR is included in version 1.7.0 |
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.
No description provided.