Skip to content

test: hold the shape rule to this ref - #102

Merged
michen00 merged 2 commits into
mainfrom
test/shape-rule-on-this-ref
Sep 15, 2026
Merged

michen00 merged 2 commits into
mainfrom
test/shape-rule-on-this-ref

Conversation

@michen00

Copy link
Copy Markdown
Owner

What changes, and why

The scheduled Smoke run has been red on all three platforms. It is not a defect in the published release, and it is not a defect in the corpus.

smoke.yml takes the harness from this ref and then replaces corpus/ with the release under test:

rm -rf corpus; git checkout "refs/tags/${TAG}" -- corpus

test_no_cli_case_ships_a_redundant_expected_tree, added by #96, is the one test in that module that asserts a property of the corpus as authored rather than of the tool's output. The tag's corpus predates that rule, so the run reported twenty cases that were correct when v0.4.0 was published. The same twenty on every platform, which is what pointed at the corpus rather than at anything platform-specific.

The smoke job now declares CORPUS_FROM_TAG, and the rule stands aside when it is set. It is keyed on where the corpus came from rather than on the existing REQUIRE_INSTALLED_PACKAGE, which would also have worked: the rule yields to the corpus's provenance, not to what is installed.

The second test pins the declaration. Removing CORPUS_FROM_TAG from the workflow fails nothing until the next scheduled run a week later, which is how this arrived as a stale red in the first place. Verified by deleting the line locally and watching the guard fail.

This does not unblock v0.5.0. Smoke cannot gate a publish by design, it is not a required context, and at release time the tag is this ref so the corpus step is a no-op. The value is that the weekly job goes back to meaning something.

Corpus

The corpus is the specification, and both implementations answer to it. Tick what applies.

  • This changes no behavior the corpus specifies.
  • This changes what gets joined, and a case in corpus/ pins the new behavior. The case was written first and failed first.
  • The change makes the tool join more than it did. The section above says what it leaves alone.

Checks

  • make check passes, or make test does and this touches no Rust.

make test — 585 passed. make tidy — every hook passed, including actionlint and yamllint over the edited workflow. Touches no Rust, so the Rust contexts cover the rest.

The skip was checked in both directions rather than inferred from a green suite: without CORPUS_FROM_TAG the rule runs and passes, with it set the rule skips and the guard still passes.

🤖 Generated with Claude Code

The scheduled smoke run takes `corpus/` from the release under test and
the harness from this ref, so the rule that a case equal to its tree
must declare unchanged was read against v0.4.0's corpus, which predates
the declaration. All three platforms reported the same twenty cases,
each correct when that version was published.

The tier now states where its corpus came from, and the rule stands
aside when it came from somewhere else. A test pins that statement,
since dropping it fails nothing until the next scheduled run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Skip corpus-shape rule for tagged smoke fixtures

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Mark smoke runs whose corpus comes from the release tag.
• Skip current-ref corpus-shape validation for historical release fixtures.
• Guard the workflow provenance declaration with an immediate regression test.
Diagram

sequenceDiagram
  participant S as Smoke Job
  participant T as Tag Corpus
  participant H as CLI Harness
  participant R as Shape Rule
  S->>T: Check out corpus
  S->>H: Set provenance flag
  H->>R: Evaluate rule
  alt Corpus from tag
    R-->>H: Skip validation
  else Current corpus
    R-->>H: Validate shape
  end
  H->>S: Verify declaration
Loading
High-Level Assessment

The explicit CORPUS_FROM_TAG signal is the best approach because it describes fixture provenance directly rather than inferring it from the installed package or Git state. Reusing REQUIRE_INSTALLED_PACKAGE would conflate artifact selection with corpus origin, while runtime inference would add unnecessary Git-dependent complexity; the guard test appropriately prevents silent workflow drift.

Files changed (2) +30 / -0

Tests (1) +24 / -0
test_cli_corpus.pySkip shape validation for historical corpora and guard workflow configuration +24/-0

Skip shape validation for historical corpora and guard workflow configuration

• Reads CORPUS_FROM_TAG and skips the redundant expected-tree rule when fixtures predate that rule. Adds a regression test asserting that the smoke workflow both checks out tagged corpus data and declares its provenance.

tests/test_cli_corpus.py

Other (1) +6 / -0
smoke.ymlDeclare tagged-corpus provenance for smoke runs +6/-0

Declare tagged-corpus provenance for smoke runs

• Defines CORPUS_FROM_TAG at the smoke-job level so the current harness knows its corpus was restored from the release tag. This prevents current corpus-authoring rules from judging historical fixtures.

.github/workflows/smoke.yml

@codecov-commenter

codecov-commenter commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.70%. Comparing base (514533e) to head (c4fb91a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #102   +/-   ##
=======================================
  Coverage   76.70%   76.70%           
=======================================
  Files           8        8           
  Lines        1185     1185           
=======================================
  Hits          909      909           
  Misses        276      276           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qodo-code-review

qodo-code-review Bot commented Sep 15, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. A misplaced smoke flag goes undetected 🐞
Description
test_the_registry_tier_says_its_corpus_is_not_from_here checks only whether the raw workflow text
contains CORPUS_FROM_TAG:, without verifying that the key belongs to the smoke job environment.
Moving the declaration to an unrelated step would leave this guard green while the corpus-shape test
runs against the tagged corpus and makes the next scheduled smoke run red.
Code

tests/test_cli_corpus.py[562]

+    assert 'CORPUS_FROM_TAG:' in text, 'the tier does not say where its corpus is from'
Evidence
The added assertion at tests/test_cli_corpus.py[559-562] searches the complete file as
unstructured text, while the behavior depends specifically on the job-scoped declaration at
.github/workflows/smoke.yml[53-60]. The repository already provides structured workflow loading
and environment traversal helpers at tests/test_workflow_contracts.py[95-129], demonstrating that
the required scope can be checked directly.

tests/test_cli_corpus.py[559-562]
.github/workflows/smoke.yml[53-60]
tests/test_workflow_contracts.py[95-129]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new workflow guard uses a substring search, so it passes when `CORPUS_FROM_TAG` exists anywhere in the file rather than specifically in the `smoke` job environment inherited by the pytest steps.

## Fix Focus Areas
- tests/test_cli_corpus.py[551-562]
- tests/test_workflow_contracts.py[95-129]
- .github/workflows/smoke.yml[53-60]

## Recommended Fix
Parse `smoke.yml` with the repository's existing workflow YAML loader and assert that `jobs.smoke.env.CORPUS_FROM_TAG` has the expected value. Prefer moving this assertion into `test_workflow_contracts.py`, where workflow parsing and contract checks already live.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 15 rules
Review mode: ⚖️ Balanced: This changes CI workflow environment propagation and conditionally alters a corpus-validation test, creating cross-file behavioral risk that merits a complete single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread tests/test_cli_corpus.py Outdated
The guard searched the workflow text for the name, so the declaration
could move to a step the pytest steps do not inherit from and the guard
would stay green while the rule it protects went back to reading the
tag's corpus. Review caught it.

It now parses the workflow and reads the value off the smoke job, which
is the level every step inherits. Moving the same name to another step
fails it. The assertion moves to the contracts module, where the loader
and the rest of the workflow wiring already live, and the corpus test
keeps a pointer to it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@michen00
michen00 merged commit 03a6794 into main Sep 15, 2026
26 checks passed
@michen00
michen00 deleted the test/shape-rule-on-this-ref branch September 15, 2026 11:32
@michen00 michen00 mentioned this pull request Sep 15, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants