Skip to content

ci: check oracle markers in tests - #2141

Merged
Benoît Cortier (CBenoit) merged 19 commits into
masterfrom
cbenoit-oracle-marker-check
Oct 22, 2026
Merged

Benoît Cortier (CBenoit) merged 19 commits into
masterfrom
cbenoit-oracle-marker-check

Conversation

@CBenoit

@CBenoit Benoît Cortier (CBenoit) commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Test markers name the source an expected value comes from, but only reviewers checked them, so a typo, a stale section, an unpinned link into another repository, or a forgotten scaffold marker could reach master unnoticed.

A new oracle markers job, beside the tests and gating the pull request through Success, runs ci/check-oracle-markers.ps1 over the tracked Rust, C#, and PowerShell files, where a Pester test is an It block and its marker a # oracle: comment. It enforces the marker grammar of test-oracle-reviewer: known kinds, property laws by name, no scaffold, and regressions as an issue or a tracked fuzz crash file. It resolves each contract, intent and docs/ spec section by anchor, heading text or heading number in a tracked file, and rejects an anchor id repeated within such a file. A citation into another repository must pin a commit of at least 12 hex digits; the file is fetched at that commit through the GitHub API and resolved the same way. When the token cannot read that repository, the reference gets only a syntax check, while a bad token or a rate limit fails the job rather than skip every pin.

On a pull request it also requires a marker on each new test. A name that names no more tests than at the base gains none, so moving a test or editing its attributes does not count; when a name does gain one, a test the diff adds is new unless it exactly matches, marker included, a base occurrence the diff moved away, and a test the check cannot tell apart counts as new, so a doubt costs a marker rather than letting an unmarked test through.

A weekly workflow runs the script with -DriftReport, which compares each pinned section with its repository's default branch and lists the pins to bump and the tests to re-check, without gating anything.

ci/check-oracle-markers.intent.md records these rules beside the script, so a later change cannot quietly undo one, and the script's Pester tests, run in their own job, cite its sections in their markers.

Since every citation names its file with the anchor, the file is the anchor's namespace: intent-files now names an anchor after its section alone and asks only that it be unique within its own document, and intent-review and contract-review report an id repeated within a file.

test-oracle-reviewer now states the marker as a line comment of the test's language, since a single // form would lead a Pester test to a comment PowerShell never reads.

🤖 Generated with Claude Code

Test markers name the source an expected value comes from, but only
reviewers checked them, so a typo, a stale section, an unpinned link
into another repository, or a forgotten scaffold marker could reach
master unnoticed.

The preflight job now runs ci/check-oracle-markers.ps1 over the
tracked Rust and C# files. It enforces the marker grammar of
test-oracle-reviewer: known kinds, property laws by name, no scaffold,
and regressions as an issue or a tracked fuzz crash file. It resolves
each contract, intent and docs/ spec section by anchor, heading text
or heading number in a tracked file. A citation into another
repository must pin a commit of at least 12 hex digits; the file is
fetched at that commit through the GitHub API and resolved the same
way. When the token cannot read that repository, the reference gets
only a syntax check, while a bad token or a rate limit fails the job
rather than skip every pin.

On a pull request it also requires a marker on each new test, a test
being new when its function name is declared more often than at the
base, so moving a test or editing its attributes does not count.

A weekly workflow runs the script with -DriftReport, which compares
each pinned section with its repository's default branch and lists
the pins to bump and the tests to re-check, without gating anything.

Pester tests for the script run in their own job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test-oracle-reviewer now says what CI enforces beyond the grammar: a
cited file must be committed, a fuzz crash file force-added past
fuzz/.gitignore, and a pin the token cannot read gets only a syntax
check. It also says when a test counts as new and that a weekly
report lists pinned sections that changed upstream.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 21, 2026 14:09
@CBenoit

Benoît Cortier (CBenoit) commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Lines changed (10 files, +1526/−7):

Part Files Added Removed
Production code ci/check-oracle-markers.ps1 774 0
Tests ci/check-oracle-markers.Tests.ps1 619 0
Intent ci/check-oracle-markers.intent.md 63 0
CI workflows .github/workflows/ci.yml, .github/workflows/oracle-marker-drift.yml 64 1
Skills test-oracle-reviewer, intent-files, intent-review, contract-review, intent-drift-audit 6 6

Note

Human-tuned, LLM-assisted content.

@CBenoit

Copy link
Copy Markdown
Member Author

UNTESTED: properties in this change:

  • $GitHubApi default block (ci/check-oracle-markers.ps1, the param block): the Pester tests replace it with a fake API, so nothing checks that it sends the token as a bearer header and returns an error status, such as 401 or 404, instead of throwing. After changing it, run a pinned marker against a public repository with a valid token, an invalid one, and none. That was done by hand for this change: a valid token and no token both resolve the section, and an invalid token stops the check with returned 401: Bad credentials.

Note

Human-tuned, LLM-assisted content.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Parsing and caching errors can accept invalid citations or reject correctly marked tests.

5 open issues
What changed in this PR

Adds automated oracle-marker validation to CI so tests cite traceable sources, plus weekly reporting of upstream changes.

Changes:

  • Validates marker grammar, references, commit pins, and markers on new tests.
  • Adds Pester coverage and a required CI test job.
  • Documents enforcement and schedules drift reports.
File Description
ci/​check-oracle-markers.Tests.ps1 Tests validation, references, drift, and new-test detection.
ci/​check-oracle-markers.ps1 Implements marker validation and drift reporting.
.github/​workflows/​oracle-marker-drift.yml Schedules weekly drift reports.
.github/​workflows/​ci.yml Runs validation and gates success on Pester tests.
.agents/​skills/​test-oracle-reviewer/​SKILL.md Documents automated enforcement.

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Remote file and outline caches now compare paths ordinally, so a
citation with the wrong case no longer reuses another's result. A code
fence closes only on its own delimiter repeated at least as often, so
headings inside nested examples stay code. An anchor names a section
only when a heading follows it, so a stale anchor no longer resolves.

New-test detection now reads whole attribute blocks, including ones
spanning lines, and bounds each test at the end of the item it
decorates, so a multiline case no longer splits a test and a marked
helper after an unmarked test no longer counts as its marker.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every citation names its file with the anchor, as in
`intent <path> §<anchor>`, so an id repeated in another file never makes
one ambiguous; only a repeat within the same file does. intent-files
now asks anchor ids to be unique within their own document only, and
intent-review and contract-review report an id repeated within a file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The skill teaches the marker grammar; how CI enforces it belongs to the
check. The one fact a contributor needs at the moment it bites, that a
fuzz crash file must be force-added past fuzz/.gitignore, now sits in
the check's error message instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An anchor id is unique within its own file and every citation names
that file, so the file is already the namespace and a scope prefix
only repeats it, as in `CONTRACT.agent-identity.md §agent-identity-channel`.
intent-files now defines the anchor as a short kebab-case name of the
section, and the intent-drift-audit example follows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The check's rules were decided in review and lived only in its code and
commit messages, so a later change could quietly undo one, such as
skipping a pin on any API error or making the drift report a gate.
The intent file records them beside the script, each section anchored
so the script's own tests can cite the rule they check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PowerShell tests can copy their expected values from the code under
test as easily as Rust and C# tests, so the check covers them from the
start. A PowerShell marker is a `# oracle:` comment, and a Pester test
is an `It` block, found with the PowerShell parser because the
hashtables of `-ForEach` cases defeat brace counting. New Pester tests
are detected by `It` name, as Rust and C# tests are by function name.

The check also fails on an anchor id repeated within a file a marker
may cite, since a citation of it could resolve to either section.

The check's own tests now carry markers citing the intent file, or
CommonMark where Markdown decides the expected value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Benoît Cortier (CBenoit) commented Oct 21, 2026 •

Copy link
Copy Markdown
Member Author

Intent drafts: handoff

Read at fd4a745c on cbenoit-oracle-marker-check (PR #2141).
Drafted on delegation ("bootstrap based on all I said so far") and approved as a whole ("I also reviewed the intent file, it's good, I approve it"); rows 32 to 35 were added after the approval and adopted separately ("The four additions are fine").

Provenance

# Commitment as written Human's words (if tidied) Level
1 The check enforces the markers test-oracle-reviewer teaches; the rules and the skill are one rule, and a difference is a defect. — 3
2 Whether the cited source decides the value is not checked; that stays with review. — 3
3 The check covers tests in Rust, C#, and PowerShell (Pester). "I would suggest we support Pester from day one." 2
4 Marker is a line comment of the language: // in Rust and C#, # in PowerShell. — 3
5 A marker sits on a test, one case, or one assertion. "on a test, a case row, or a single assertion" 2
6 A Pester test is an It block; a case is one -ForEach or -TestCases entry. — 3
7 Only tracked files are read, because ignored or uncommitted files exist locally but not in CI. "apply the review fixes (tracked files only, …)" 3
8 The kinds are exactly the eight. "Kinds: spec … mutant, scaffold (weak)" 2
9 A spec names a document; a spec under docs/ takes its path instead. "spec <name> §<section>: a named spec (RFC 9421, X.690…). A spec under docs/ takes its path instead." 2
10 A spec name that looks like a URL, an @ ref, or a file name outside docs/ is rejected. — 3
11 A regression names a Jira key, #123, owner/repo#123, or a committed crash file under fuzz/. "regression <Jira key | #123 | owner/repo#123 | crash file under fuzz/>" 2
12 A property names exactly one of the six laws; anything else is rejected. "the law is exactly one of … Reject anything else." 2
13 Any scaffold marker fails the check, because none may merge. "Fail on any scaffold: none may merge." 1
14 Paths from the repository root, forward slashes, exact case. "the path is from the repository root, forward slashes, exact case." 1
15 § resolution order: exact anchor id, heading text in any case, heading number prefix. the three numbered steps 2
16 An anchor names the heading that follows it; an anchor with no heading after it names nothing. #2136 skill text: "an anchor the file places before a heading" 2
17 Fails when the path does not exist or nothing matches. "Fail when nothing matches, or the path doesn't exist." 1
18 An anchor id is unique within its own document, and only there; a repeat in one file a marker may cite fails, the same id in two files is allowed. "I think an anchor should only be unique within its own document"; "unique within its own document is all we need" 2
19 Cross-repository form <owner>/<repo>@<sha>:<path> §<section>, path-taking kinds only. "Cross-repository references (path-taking kinds only)" 2
20 SHA of at least 12 hex digits; branches and tags rejected, because they move. "Reject branches and tags, because they move." 1
21 Unpinned cross-repository references fail. "Unpinned cross-repository forms fail." 1
22 Fetched at that commit, resolved by the same rules; deterministic. "A commit never changes, so this is deterministic." 2
23 Cached by <owner>/<repo>@<sha>:<path>. "Cache by owner/repo@sha:path." 1
24 Without the privilege to read the repository, only a syntax check. "If it's not possible to fetch the file at that commit because the privilege is missing, I'm good with simply skipping the check, at which point it's just a syntax check" 2
25 A bad token or a rate limit fails rather than skip. — 3
26 On a pull request, every new test carries a marker. "new tests carry a marker" 2
27 An existing unmarked test blocks nothing; moving or editing a test does not make it new, unless its name also gains a test in the same change. "apply the review fixes (… new tests by function name …)"; "Accept the gap, but in favor of rejecting." 3
36 A test the check cannot tell from an edited or moved one counts as new, so a doubt costs a marker. "Accept the gap, but in favor of rejecting." 2
28 Drift report: scheduled, not a pull-request gate; compares the pinned section with the default branch. "Drift report (scheduled, not a pull-request gate)" 2
29 Unchanged → proposed mechanical pin bump; changed, moved, or removed → listed for a person to re-check. "Unchanged → propose a mechanical pin bump. Changed, moved or removed → list the test for a person to re-check against the new text." 2
30 One table per repository: test, marker, status. "Output one table per repository: test, marker, status." 1
31 A PowerShell script, not an xtask, because it runs with no build step ahead of every other CI job. chose "Keep PowerShell" over "Move to xtask"; the reason is agent wording 3

| 32 | contract takes a path to a CONTRACT*.md file, intent to an INTENT*.md or *.intent*.md file. | "The four additions are fine" | 3 |
| 33 | spec, contract, and intent cite a section as <document> §<section>. | "The four additions are fine" | 3 |
| 34 | Every kind but scaffold takes a reference. | "The four additions are fine" | 3 |
| 35 | The repeated-anchor rule covers files a marker may cite, under docs/ or a contract or intent file. | "The four additions are fine" | 3 |

Left out on purpose

  • Exact drift statuses beyond the three (current, skipped, unresolved) and what distinguishes moved from changed (same body, new heading): implementation of row 29.
  • Running in preflight, the token secret and its fallback, the weekly schedule, -Base HEAD^1: CI wiring, not a property of the check.
  • New-test detection by function or It name count: the mechanism behind row 27.
  • The repository-wide anchor-uniqueness rule: dropped from intent-files in the same pull request, since ids need only be unique within their own document; anchors are now named after their section alone.
  • Markdown rules the check follows, such as fenced code and line endings: CommonMark decides them, and the tests cite it directly.
  • Anything naming a particular private repository: kept out at your request.

Placement

One file, ci/check-oracle-markers.intent.md, beside the script it governs; every section carries an anchor so the script's tests can cite it.

Pending experiments

None.

Note

Human-tuned, LLM-assisted content.

Pester evaluates `-ForEach` data during discovery, before `BeforeAll`
runs, so fixture rows that spelled the marker through a variable set
there carried none. They now write it as "`# oracle:", which builds the
marker without the tests file itself holding one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A marker is a line comment of the test's language, so one sentence
covers Rust, C#, and PowerShell alike, and naming a single comment
syntax would lead a Pester test to a `//` that PowerShell never reads as
a comment. The exact forms stay in the oracle marker check's intent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
A test's end is now found on code with comments and string literals
blanked across lines, and a semicolon ends an item only outside
parentheses and brackets, so a brace in a comment or raw string, or a
fixed-size array parameter, no longer moves the end of a test.

A name's new tests in a file are counted as the lesser of how much more
often the file and the tree declare it than at the base, so an existing
unmarked test stays exempt when a namesake joins its file; when markers
fall short, the unmarked tests the diff adds are reported first.

Pester names are counted regardless of the case of `It`, which
PowerShell ignores, and a bare Markdown file name is rejected as a spec
name, since it is a path that would escape resolution.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The gate can accept unmarked tests, reject valid moves or string fixtures, and inconsistently enforce marker and anchor rules.

1 open issue
3 previously missed issues

In code that hasn't changed since last review

Medium severity Extract markers from comments instead of raw source lines

ci/​check-oracle-markers.ps1:49

[P2] Extract markers from actual comments, not raw source lines. A Rust multiline raw string or PowerShell here-string containing a standalone // oracle: property round-trip or # oracle: property round-trip makes an otherwise unmarked new test pass. Literal fixture text containing a scaffold marker instead fails CI. C# multiline strings have the same problem. Both Test-Markers and $Marked inspect the original text; literal blanking only protects test-boundary detection. Share language-aware comment extraction between validation and marker ownership, preserve the supported Rust doctest form, and add fixtures for marker-looking string data.

Medium severity Enforce anchor uniqueness across all fetched files

ci/​check-oracle-markers.ps1:281

[P2] Apply document-wide anchor uniqueness to fetched files too. This condition rejects duplicate IDs only when the citation names that ID. In the pinned repeated-anchor fixture, changing §x to the heading §A makes the same document pass. That conflicts with ci/check-oracle-markers.intent.md §paths and the local Test-Anchors check. Reject any nonempty duplicate list, report the offending ID, and add a pinned-file case cited by heading or number.

Medium severity Make marker casing consistent with validation scans

ci/​check-oracle-markers.ps1:574

[P2] Use the same marker casing rules as the validation scan. PowerShell's -match is case-insensitive, but Test-Markers uses case-sensitive git grep. A new test containing only // ORACLE: scaffold therefore counts as marked here while its marker is never validated, so the test passes CI. Use -cmatch to agree with the lowercase validation scan, and add a fixture showing that an uppercase label does not satisfy the marker requirement.

5 resolved since last review

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
Markers were read from raw source lines, so marker-shaped text inside a
multiline string counted, and a label in another case satisfied the
new-test rule without ever being validated. A marker is now taken only
from a line comment, matched case-sensitively, through the PowerShell
tokenizer or the Rust and C# literal scan, for validation and for the
new-test rule alike.

A name's new tests are now among the tests the diff adds, and one that
matches a base occurrence of its name, being a moved test, neither
needs a marker nor lends one. An existing marked namesake therefore no
longer covers a new unmarked test, and moving an unmarked test while a
marked namesake is added elsewhere no longer flags the moved one.

A pinned file that a marker cites now fails on any repeated anchor,
whatever section the citation names, as a local one already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Copy link
Copy Markdown
Member Author

The three previously missed findings from the review of 808abb3, which have no inline thread, are fixed in 845b463:

  • Markers from comments only: markers are now taken from line comments, through the PowerShell tokenizer or the Rust and C# literal scan, for validation and for the new-test rule alike, so marker-shaped text in a Rust raw string, a C# verbatim string, or a PowerShell here-string is data; the Rust doctest form /// # // oracle: still counts.
  • Repeated anchors in fetched files: a pinned file a marker cites now fails on any repeated anchor, whatever section the citation names, with a case citing it by heading.
  • Marker case: markers match case-sensitively everywhere, so // ORACLE: is neither validated nor credited, with cases for both.

Note

Human-tuned, LLM-assisted content.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Occurrence matching can miss unmarked additions, literal scanning can cause false CI failures, and section boundaries can produce false drift reports.

3 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Exclude next section anchors from the compared body

ci/​check-oracle-markers.ps1:175

[P2] Exclude the next section's anchors from the compared body.
$End points at the next heading, so an anchor immediately above that heading is included in the preceding section's body.
Adding an anchor to sibling section B therefore reports unchanged section A as changed and asks for its test to be re-checked instead of proposing a pin bump.
End the body before the anchor-only and blank lines attached to the next heading, and add a drift fixture where only B gains an anchor.

1 resolved since last review

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
Rust block comments nest and C# ones do not, so the literal scan is
now per language: text still inside an outer Rust comment is no
marker, while a C# comment ends at its first `*/`. A C# interpolation
hole is read as code holding its own strings and braces, so a `"}"`
inside one no longer ends a test early.

A section now ends before the anchors and blank lines attached to the
next heading, so an anchor added to a sibling section no longer makes
the drift report call an unchanged section changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Copy link
Copy Markdown
Member Author

The previously missed finding from the review of 845b463, which has no inline thread, is fixed in 0a37ab1: a section now ends before the anchors and blank lines attached to the next heading, with a drift case where only the next section gains an anchor and the pinned one is reported unchanged.

Note

Human-tuned, LLM-assisted content.

Telling an edited or moved test from a new namesake has no exact
answer without a stable identity for tests, and lending one test's
marker to another let unmarked tests through. A test the diff adds is
now new unless it exactly matches, marker included, a base occurrence
of its name that the diff moved away, and each new test needs its own
marker. When the check cannot tell, it counts the test as new, so a
doubt costs a marker rather than letting an unmarked test through.

The intent's new-test rules now say so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Test discovery and name handling can allow new unmarked tests to bypass the CI gate.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Preserve quoted Pester names containing apostrophes

ci/​check-oracle-markers.ps1:65

[P2] Preserve complete quoted Pester names.
Both declaration patterns stop at either quote, so It "doesn't mutate input" and It "doesn't retry a denial" are both named doesn.
Replacing the former with the latter leaves the declaration counts unchanged, and the $Gain -le 0 check skips the new unmarked test.
Respect the opening quote and PowerShell escaping when extracting names, and use the same identity for declaration counts and base-fingerprint searches.
Add a replacement-test fixture with an apostrophe inside a double-quoted name.

3 resolved since last review

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
Comment thread ci/check-oracle-markers.ps1 Resolved
A declaration that follows its attributes on their line, as in
`[Fact] public void A() { }`, now opens its own test, so two one-line
tests no longer merge and lend each other a marker. Test names group
case-sensitively, as their counts do, and a Pester name is read to its
own closing quote with doubled quotes as escapes, so names that differ
in case or after an apostrophe stay distinct.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Copy link
Copy Markdown
Member Author

The previously missed finding from the review of 476b3a8, which has no inline thread, is fixed in d466460: a Pester name is read to its own closing quote, with doubled quotes as escapes, in the name counts and the base search alike, with a case replacing It "doesn't mutate input" by an unmarked It "doesn't retry a denial".

Note

Human-tuned, LLM-assisted content.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Parsing errors can let unmarked new tests pass and reject correctly marked tests or valid Markdown documents.

1 open issue
3 previously missed issues

In code that hasn't changed since last review

Medium severity Detect test attributes anywhere in C# attribute lists

ci/​check-oracle-markers.ps1:58

[P2] Recognize test attributes anywhere in a C# attribute list.
A valid declaration such as [Trait("Category", "Unit"), Fact] never sets $Attribute, because this pattern requires Fact or Theory immediately after [, and Get-Tests only checks it when entering the list.
The test is therefore omitted, so a newly added unmarked fact passes the gate.
Inspect top-level attribute entries across the complete list, including multiline lists, and add fixtures with Fact or Theory after another attribute.

Medium severity Handle backtick-escaped quotes in Pester test names

ci/​check-oracle-markers.ps1:70

[P2] Handle backtick-escaped quotes in Pester names.
Both PowerShell declaration patterns stop at a double quote even when it is escaped with a backtick.
Replacing It "accepts `"allow`" policy" with It "accepts `"deny`" policy" therefore truncates both names to the same prefix.
The declaration-count gain is zero, so the newly named test passes without a marker.
Consume backtick escapes in both patterns and add this replacement case alongside the apostrophe fixture.

Medium severity Exclude Markdown code spans from anchor ID collection

ci/​check-oracle-markers.ps1:132

[P2] Exclude Markdown code spans when collecting anchor IDs.
A paragraph showing <a id="channel"></a> inside backticks adds channel to $Seen, so a later real anchor with that ID triggers a duplicate violation.
A document with only one rendered anchor therefore blocks preflight; the same scan also rejects valid pinned files.
Exclude code-span content from anchor extraction and add a fixture that both documents and declares the same anchor.

2 resolved since last review

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
A test's body now opens only at a brace outside parentheses and
brackets, so an array initializer in an attribute such as
`InlineData(new int[] { 1, 2 })` no longer ends a theory before its
body. A C# test attribute is found anywhere in its attribute list,
including a continuation line, as in `[Trait("Category", "Unit"),
Fact]`. A double-quoted Pester name reads past a backtick-escaped
quote, so names that differ only after one stay distinct. An anchor
shown in a Markdown code span is no anchor, so a document that shows
and declares the same one is not a duplicate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Copy link
Copy Markdown
Member Author

The three previously missed findings from the review of d466460, which have no inline thread, are fixed in 4a32562:

  • Test attributes anywhere in a C# list: Fact or Theory is found after another attribute in its list, including on a continuation line, with cases for [Trait("Category", "Unit"), Fact] and a Theory on the list's second line.
  • Backtick-escaped quotes in Pester names: a double-quoted name reads past `", in the name counts and the base search alike, with a case replacing It "accepts `"allow`" policy" by an unmarked It "accepts `"deny`" policy".
  • Anchors in code spans: an anchor shown in a Markdown code span is not collected, with a case of a document that shows <a id="channel"></a> in backticks and declares it, cited by a marker.

Note

Human-tuned, LLM-assisted content.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The declaration-counting gap needs correction, and the custom cross-language parser and untested live GitHub adapter warrant human validation before becoming a CI gate.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
Declaration counts covered every function and method, so adding a test
attribute to an existing helper, or replacing a deleted helper with a
same-named test, left a name's count unchanged and let the new test
through unmarked. A name's gain is now the change in the tests it
names, found in the files that declare it at the base and now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In preflight, a misread marker stopped every other job; in its own job
beside the tests, it still gates the pull request through Success while
the rest of CI runs and reports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved test-detection and occurrence-matching errors undermine the gate, and drift reporting can recommend a pin that CI rejects.

1 open issue
3 previously missed issues

In code that hasn't changed since last review

Medium severity Exclude qualified rstest fixtures from test detection

ci/​check-oracle-markers.ps1:58

[P2] Exclude qualified rstest fixtures from test detection.
#[rstest::fixture] matches this pattern because : satisfies the word boundary after rstest.
Adding a fixture therefore makes Get-Tests and Get-NamedTests count a helper as a new test, and CI rejects it for lacking an oracle marker.
Recognize rstest and rstest::rstest explicitly and check the attribute-name terminator, preserving attributes whose closing bracket is on the next line.
Add a fixture-exclusion case alongside the test-attribute cases.

[!NOTE]
LLM-assisted content (no human feedback).

Medium severity Consume retained occurrences before allocating move fingerprints

ci/​check-oracle-markers.ps1:639

[P2] Consume retained base occurrences even when their bodies change.
Suppose the base has unmarked a::same and marked b::same: edit only a separate body line in a, change b's test attribute, and add unmarked c::same copied from the original a.
Because a's attribute and declaration lines are unchanged, it enters $Kept, but its changed fingerprint removes nothing from $Vacated.
With two added candidates and a name-count increase of one, c consumes a's old fingerprint as a move; the check passes because the remaining candidate, b, is marked.
Match retained tests to their base occurrences using unchanged attribute/declaration locations before allocating move fingerprints, and add this body-edit variant to the namesake fixtures.

[!NOTE]
LLM-assisted content (no human feedback).

Medium severity Detect duplicate anchors before recommending a pin bump

ci/​check-oracle-markers.ps1:706

[P2] Check default-branch anchor uniqueness before recommending a pin bump.
If upstream repeats the cited anchor before another section but leaves the original section unchanged, Find-Section still selects the first occurrence and the report recommends unchanged: bump the pin.
Following that recommendation fails CI because Test-CrossRepositoryReference rejects any duplicate anchor in the pinned file.
Report the ambiguity as requiring a re-check instead of proposing a mechanical bump, and add a drift fixture for this upstream change.

[!NOTE]
LLM-assisted content (no human feedback).

1 resolved since last review

Review effort: Balanced

Comment thread ci/check-oracle-markers.ps1 Resolved
`[FactAttribute]` and `[Xunit.TheoryAttribute]` now count as tests, as
`[Fact]` and `[Theory]` do, while `#[rstest::fixture]` no longer does,
since only `#[rstest]` and `#[rstest::rstest]` declare an rstest test.

The drift report asks for a re-check rather than a pin bump when the
cited file repeats an anchor on the default branch, since a pin into
that file would fail the check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CBenoit

Copy link
Copy Markdown
Member Author

The three previously missed findings from the review of 2c7af74, which have no inline thread:

  • Qualified rstest fixtures: fixed in 1426a58. Only #[rstest] and #[rstest::rstest] declare an rstest test, so #[rstest::fixture] is no test, with a case adding qualified and plain fixtures.
  • Repeated anchors upstream: fixed in 1426a58. The drift report asks for a re-check rather than a pin bump when the cited file repeats an anchor on the default branch, with a drift case.
  • Retained occurrences with edited bodies: not changed. The case needs three same-named tests in one change, one with only its body edited, another with its attribute changed, and an unmarked copy of the first; matching tests by location to cover it is more machinery than this case warrants today. Every test is meant to carry a marker eventually, and review still asks for one on each changed test, so a test this rare layout lets through is caught there.

Note

Human-tuned, LLM-assisted content.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The required gate has an unresolved test-detection gap, custom language parsing, and an explicitly untested live GitHub API adapter that warrant human review.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Detect executable Rust doctests in new-test marker checks

ci/​check-oracle-markers.ps1:469

[P2] Include executable Rust doctests in the new-test check.
Get-CodeLines blanks rustdoc comments, and Get-Tests only collects attribute-decorated items, so a newly added rustdoc code block containing assertions produces no test record and passes -Base without a marker.
These blocks run through cargo test --workspace in ci/tlk.ps1:949–968, but escape the requirement in ci/check-oracle-markers.intent.md §new-tests that every new test carry a marker.
Detect executable rustdoc blocks separately, compare them against the base, and add fixtures for marked and unmarked additions while excluding non-test code fences.

[!NOTE]
LLM-assisted content (no human feedback).

1 resolved since last review

Review effort: Balanced

@CBenoit

Copy link
Copy Markdown
Member Author

The previously missed finding from the review of 1426a58, Rust doctests in the new-test check, is answered without a change: only ten files here hold rustdoc code blocks, and detecting new executable ones means a separate detector for doc comments and their fence attributes, which is more than this case warrants today. A doctest marker is still validated when present, and review still asks for a marker on each changed test.

Copilot's final verdict, for head 1426a58:

🔵 Needs a closer look

The required gate has an unresolved test-detection gap, custom language parsing, and an explicitly untested live GitHub API adapter that warrant human review.

No open issues 🎉

The review loop stops here; the remaining judgment is the maintainer's.

Note

Human-tuned, LLM-assisted content.

@CBenoit
Benoît Cortier (CBenoit) merged commit c32e95c into master Oct 22, 2026
49 checks passed
@CBenoit
Benoît Cortier (CBenoit) deleted the cbenoit-oracle-marker-check branch October 22, 2026 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants