chore(tooling): an accepted warning raised to an error is not "resolved" (template#101)

.github/workflows/engine-validate.yml

@@ -35,6 +35,7 @@ jobs:
         # itself); the version range matches the engine's own dependency.
         run: npm install --no-save "learn-content-engine@$(cat schema/engine-version.txt)" "yaml@^2.9.0"
       - name: Self-test (the gate must bite before it may pass)
+        id: selftest
         run: node scripts/validate_with_engine.mjs --self-test
       - name: Validate all lessons and manifests with the engine
         run: node scripts/validate_with_engine.mjs .
@@ -45,12 +46,16 @@ jobs:
         # otherwise reach nobody unless someone runs `make lint-warnings` by
         # hand, so a pin bump changes a version number and nothing on screen.
         # The findings go into the job summary, readable without opening the
-        # log.
+        # log. It also runs when the gate above failed: a pin that raises an
+        # accepted warning to an error turns the gate red, and the summary is
+        # where that is explained (template#101).
+        if: ${{ !cancelled() && steps.selftest.outcome == 'success' }}
         continue-on-error: true
         run: |
-          # Keep the runner's exit code: it is 0 when it only finds warnings, so
-          # anything else is a crash, and a crash must show red instead of a
-          # green step with an empty table that reads like "no warnings".
+          # Keep the runner's exit code: 0 without errors, 1 with errors (the
+          # gate above has failed on them already), anything else a crash. A
+          # crash must show red instead of a green step with an empty table
+          # that reads like "no warnings".
           set +e
           node scripts/validate_with_engine.mjs --warnings . > /tmp/lint-warnings.txt 2>&1
           status=$?

scripts/quality_state.py

@@ -15,7 +15,11 @@
   of the elements it covers. A decision that cannot be read off the state is
   a backlog three months later: the warning summary shows the decision next to
   the number, and compares the recorded elements with the current ones, so
-  "still the same 105" is a set comparison, not a count.
+  "still the same 105" is a set comparison, not a count. An error cannot be
+  accepted: it blocks the gate. When the engine raises an accepted rule to an
+  error, its ID changes tier (``W-X`` becomes ``E-X``); the runner prints the
+  engine's rename table, so the record is still recognized under its old name
+  and the summary reports the raise instead of calling the findings resolved.
 
 Commands (all write Markdown for the job summary to stdout):
 
@@ -40,8 +44,10 @@
 TABLE_ROWS = 50
 
 FINDING_LINE = re.compile(r"^(?P<path>[^ ].*?):(?P<line>\d+): ")
-WARN_FILE = re.compile(r"^WARN (?P<path>\S+)")
-WARN_ENTRY = re.compile(r"^\s+\[(?P<rule>W-[A-Z0-9-]+)\] (?P<pointer>\S+)")
+RULE_ID = r"[EW]-[A-Z0-9-]+"
+ISSUE_FILE = re.compile(r"^(?:WARN|✗) (?P<path>\S+)")
+ISSUE_ENTRY = re.compile(rf"^\s+\[(?P<rule>{RULE_ID})\] (?P<pointer>\S+)")
+RENAMED_LINE = re.compile(rf"^RENAMED (?P<retired>{RULE_ID}) -> (?P<current>{RULE_ID})$")
 STEP_POINTER = re.compile(r"^/steps/(?P<index>\d+)/exercise")
 
 
@@ -64,6 +70,8 @@ def load_state(path: Path = STATE_PATH) -> dict:
         missing = [key for key in ("rule", "since", "reason", "ids") if key not in entry]
         if missing:
             raise ValueError(f"{path}: accepted warning {entry.get('rule', '?')} needs {', '.join(missing)}")
+        if is_error(entry["rule"]):
+            raise ValueError(f"{path}: {_refusal(entry['rule'])}")
     return state
 
 
@@ -121,17 +129,39 @@ def unchanged(self) -> bool:
         return not self.new and not self.resolved
 
 
-def parse_warnings(output: str) -> list[Finding]:
-    """Every author warning in the engine runner's --warnings output."""
+def is_error(rule: str) -> bool:
+    """Whether a rule ID names an error: its tier is its prefix, ``E-`` blocks."""
+    return rule.startswith("E-")
+
+
+def _refusal(rule: str) -> str:
+    return f"{rule} is an error: it blocks the gate and cannot be accepted"
+
+
+def parse_findings(output: str) -> list[Finding]:
+    """Every finding in the engine runner's --warnings output, warnings
+    (``WARN`` blocks) and errors (``✗`` blocks) alike. An error line without a
+    rule ID, as an older runner prints it, is skipped."""
     findings, current = [], None
     for line in output.splitlines():
-        if match := WARN_FILE.match(line):
+        if match := ISSUE_FILE.match(line):
             current = match.group("path")
-        elif current and (match := WARN_ENTRY.match(line)):
+        elif current and (match := ISSUE_ENTRY.match(line)):
             findings.append(Finding(current, match.group("rule"), match.group("pointer")))
     return findings
 
 
+def parse_renames(output: str) -> dict[str, str]:
+    """The engine's rename table as the runner prints it: every retired rule
+    ID mapped to the ID its rule carries now. Empty when the runner printed
+    none; a rename is never guessed from a shared name."""
+    return {
+        match.group("retired"): match.group("current")
+        for line in output.splitlines()
+        if (match := RENAMED_LINE.match(line))
+    }
+
+
 def element_key(finding: Finding, root: Path) -> str:
     """The identity of the element a warning is about: the exercise's
     stable_id (or its id before minting) when the pointer lands on one, else
@@ -165,21 +195,36 @@ def _status_text(status: AcceptedStatus) -> str:
     return "; ".join(parts)
 
 
+def _raised_text(current_rule: str, count: int) -> str:
+    if count == 0:
+        return f"raised to `{current_rule}`: no findings left, remove the record"
+    noun = "finding" if count == 1 else "findings"
+    return f"raised to `{current_rule}`: an error blocks the gate, the acceptance no longer applies ({count} {noun})"
+
+
 def warnings_report(output: str, state: dict, root: Path = Path(".")) -> str:
     """Markdown for the job summary: warnings per rule, each accepted rule with
-    its decision next to the number and a set comparison against the record."""
-    findings = parse_warnings(output)
+    its decision next to the number and a set comparison against the record.
+    An accepted rule the engine has since raised to an error keeps its row
+    under the recorded name and says so; other errors stay out of the table."""
+    findings = parse_findings(output)
+    renames = parse_renames(output)
     per_rule = Counter(finding.rule for finding in findings)
     accepted = {entry["rule"]: entry for entry in state.get("accepted_warnings", [])}
+    warned = {rule for rule in per_rule if not is_error(rule)}
     lines = ["### Author lints (warnings, non-blocking)", "", "| rule | count | accepted | status |", "|---|---|---|---|"]
-    for rule in sorted(set(per_rule) | set(accepted), key=lambda r: (-per_rule[r], r)):
+    for rule in sorted(warned | set(accepted), key=lambda r: (-per_rule[renames.get(r, r)], r)):
         entry = accepted.get(rule)
         if entry is None:
             lines.append(f"| `{rule}` | {per_rule[rule]} | - | |")
             continue
-        current = {element_key(f, root) for f in findings if f.rule == rule}
-        status = compare_accepted(entry, current)
-        lines.append(f"| `{rule}` | {per_rule[rule]} | {len(entry['ids'])} since {entry['since']} | {_status_text(status)} |")
+        current_rule = renames.get(rule, rule)
+        if is_error(current_rule):
+            status_text = _raised_text(current_rule, per_rule[current_rule])
+        else:
+            current = {element_key(f, root) for f in findings if f.rule == rule}
+            status_text = _status_text(compare_accepted(entry, current))
+        lines.append(f"| `{rule}` | {per_rule[current_rule]} | {len(entry['ids'])} since {entry['since']} | {status_text} |")
     if accepted:
         lines += ["", "**Accepted warnings**", ""]
         for rule, entry in sorted(accepted.items()):
@@ -189,13 +234,18 @@ def warnings_report(output: str, state: dict, root: Path = Path(".")) -> str:
 
 
 def accept_warnings(state: dict, rule: str, output: str, root: Path, reason: str, since: str, decision: str | None = None) -> dict:
-    """The state with the current findings of ``rule`` recorded as accepted."""
-    ids = sorted({element_key(f, root) for f in parse_warnings(output) if f.rule == rule})
-    entry = {"rule": rule, "count": len(ids), "since": since, "reason": reason}
+    """The state with the current findings of ``rule`` recorded as accepted,
+    under the ID the rule carries now. Raises ValueError for an error, also for
+    the old name of a rule the engine has since raised to one."""
+    current_rule = parse_renames(output).get(rule, rule)
+    if is_error(current_rule):
+        raise ValueError(_refusal(current_rule))
+    ids = sorted({element_key(f, root) for f in parse_findings(output) if f.rule == current_rule})
+    entry = {"rule": current_rule, "count": len(ids), "since": since, "reason": reason}
     if decision:
         entry["decision"] = decision
     entry["ids"] = ids
-    kept = [e for e in state.get("accepted_warnings", []) if e["rule"] != rule]
+    kept = [e for e in state.get("accepted_warnings", []) if e["rule"] != current_rule]
     return {**state, "accepted_warnings": kept + [entry]}
 
 
@@ -225,12 +275,15 @@ def main(argv: list[str]) -> int:
     if args.command == "warnings":
         sys.stdout.write(warnings_report(args.output.read_text(encoding="utf-8"), state))
         return 0
-    updated = accept_warnings(state, args.rule, args.output.read_text(encoding="utf-8"), Path("."),
-                              reason=args.reason, since=args.since, decision=args.decision)
+    try:
+        updated = accept_warnings(state, args.rule, args.output.read_text(encoding="utf-8"), Path("."),
+                                  reason=args.reason, since=args.since, decision=args.decision)
+    except ValueError as refusal:
+        parser.error(str(refusal))
     STATE_PATH.parent.mkdir(parents=True, exist_ok=True)
     STATE_PATH.write_text(json.dumps(updated, ensure_ascii=False, indent=2) + "\n", encoding="utf-8")
-    entry = next(e for e in updated["accepted_warnings"] if e["rule"] == args.rule)
-    print(f"recorded {entry['count']} accepted {args.rule} finding(s) in {STATE_PATH}")
+    entry = updated["accepted_warnings"][-1]
+    print(f"recorded {entry['count']} accepted {entry['rule']} finding(s) in {STATE_PATH}")
     return 0
 
 

scripts/validate_with_engine.mjs

@@ -29,9 +29,20 @@
  * `--warnings` also lists the author lints (W-*) that never block. It runs
  * through the SAME extension registry as the error gate, so ext: lessons are
  * validated instead of refused - `make lint-warnings` used to shell out to the
- * bare CLI (no registry) and died on ext content (content-test#71).
+ * bare CLI (no registry) and died on ext content (content-test#71). It also
+ * prints the engine's rename table (`RENAMED W-X -> E-X`): raising a rule
+ * renames it, and scripts/quality_state.py follows an accepted warning to its
+ * new ID instead of reporting it resolved (template#101). Errors carry their
+ * rule ID in both modes, in the same `[ID] path message` form as warnings.
  */
-import { validateLesson, validateLessonQuality, validateManifest, validateManifestPair } from "learn-content-engine";
+import {
+  currentRuleId,
+  RENAMED_RULE_IDS,
+  validateLesson,
+  validateLessonQuality,
+  validateManifest,
+  validateManifestPair,
+} from "learn-content-engine";
 import { readFileSync, readdirSync, statSync } from "node:fs";
 import { join, relative } from "node:path";
 import { parse as parseYaml } from "yaml";
@@ -372,14 +383,15 @@ function validateAll(repoRoot, { showWarnings = false } = {}) {
       (showWarnings ? `, ${totalWarnings} warning(s)` : ""),
   );
   if (showWarnings) {
+    for (const retired of Object.keys(RENAMED_RULE_IDS)) console.log(`RENAMED ${retired} -> ${currentRuleId(retired)}`);
     for (const w of warned) {
       console.log(`\nWARN ${w.file}`);
       for (const issue of w.warnings) console.log(`   [${issue.id}] ${issue.path} ${issue.message}`);
     }
   }
   for (const p of problems) {
     console.error(`\n✗ ${p.file}`);
-    for (const e of p.errors) console.error(`   ${e.path}: ${e.message}`);
+    for (const e of p.errors) console.error(`   [${e.id}] ${e.path} ${e.message}`);
   }
   return problems.length === 0 ? 0 : 1;
 }

tests/test_quality_state.py

@@ -114,7 +114,7 @@ def _lesson(tmp_path):
 
 
 def test_warnings_are_parsed_per_file(tmp_path):
-    findings = quality_state.parse_warnings(WARNINGS_OUTPUT)
+    findings = quality_state.parse_findings(WARNINGS_OUTPUT)
     assert [(f.path, f.rule, f.pointer) for f in findings] == [
         ("sets/de/x/lessons/01-a.json", "W-CLOZE-NO-CARRIER", "/steps/1/exercise/sentence"),
         ("sets/de/x/lessons/01-a.json", "W-CARD-UNUSED", "/cards"),
@@ -123,13 +123,13 @@ def test_warnings_are_parsed_per_file(tmp_path):
 
 def test_a_finding_on_an_exercise_is_identified_by_its_stable_id(tmp_path):
     root = _lesson(tmp_path)
-    finding = quality_state.parse_warnings(WARNINGS_OUTPUT)[0]
+    finding = quality_state.parse_findings(WARNINGS_OUTPUT)[0]
     assert quality_state.element_key(finding, root) == "ex-abc"
 
 
 def test_a_finding_without_an_exercise_falls_back_to_file_and_pointer(tmp_path):
     root = _lesson(tmp_path)
-    finding = quality_state.parse_warnings(WARNINGS_OUTPUT)[1]
+    finding = quality_state.parse_findings(WARNINGS_OUTPUT)[1]
     assert quality_state.element_key(finding, root) == "sets/de/x/lessons/01-a.json#/cards"
 
 
@@ -176,3 +176,98 @@ def test_accepting_records_the_ids_count_date_and_reason(tmp_path):
         "reason": "kept on purpose",
         "ids": ["ex-abc"],
     }
+
+
+# --- a rule raised to an error (template#101) ---------------------------------
+# Raising a rule renames it: the tier is the prefix of its ID, so 0.38.0 turned
+# W-MANIFEST-ENTRY-MISMATCH into E-MANIFEST-ENTRY-MISMATCH (engine#256). The
+# runner prints the engine's rename table (RENAMED_RULE_IDS, followed with
+# currentRuleId) and the errors with their IDs, so a record kept under the old
+# name is still recognized. This output is the example in the template README.
+
+RAISED_OUTPUT = """engine-validate: 1 lesson(s), 2 manifest(s) checked - 1 file(s) with errors, 0 warning(s)
+RENAMED W-MANIFEST-ENTRY-MISMATCH -> E-MANIFEST-ENTRY-MISMATCH
+
+✗ sets/de/x/manifest.yaml
+   [E-MANIFEST-ENTRY-MISMATCH] /sets/0/description set 'x': 'description' is "Root text" in the root manifest but "Set text" in the set manifest; the two must be identical
+"""
+
+RAISED_RECORD = {
+    "rule": "W-MANIFEST-ENTRY-MISMATCH",
+    "since": "2026-09-23",
+    "reason": "the root text is shorter on purpose",
+    "ids": ["sets/de/x/manifest.yaml#/sets/0/description"],
+}
+
+
+def test_errors_are_parsed_with_their_rule_ids():
+    [finding] = quality_state.parse_findings(RAISED_OUTPUT)
+    assert (finding.path, finding.rule, finding.pointer) == (
+        "sets/de/x/manifest.yaml",
+        "E-MANIFEST-ENTRY-MISMATCH",
+        "/sets/0/description",
+    )
+
+
+def test_the_renames_are_read_from_the_runner_output():
+    assert quality_state.parse_renames(RAISED_OUTPUT) == {"W-MANIFEST-ENTRY-MISMATCH": "E-MANIFEST-ENTRY-MISMATCH"}
+
+
+def test_without_a_rename_table_there_are_no_renames():
+    # An older runner prints no table: nothing is renamed, nothing is guessed
+    # from a shared suffix.
+    assert quality_state.parse_renames(WARNINGS_OUTPUT) == {}
+
+
+def test_an_accepted_warning_raised_to_an_error_is_not_reported_as_resolved(tmp_path):
+    # The reproduction: the record names the old ID, the findings carry the new
+    # one. Compared under the old ID, every recorded element looked resolved.
+    report = quality_state.warnings_report(RAISED_OUTPUT, {"accepted_warnings": [RAISED_RECORD]}, tmp_path)
+    assert "resolved" not in report
+    assert (
+        "| `W-MANIFEST-ENTRY-MISMATCH` | 1 | 1 since 2026-09-23 | raised to `E-MANIFEST-ENTRY-MISMATCH`:"
+        " an error blocks the gate, the acceptance no longer applies (1 finding) |"
+    ) in report
+
+
+def test_a_raised_rule_without_findings_asks_for_the_record_to_go(tmp_path):
+    output = RAISED_OUTPUT.split("\n✗")[0]
+    report = quality_state.warnings_report(output, {"accepted_warnings": [RAISED_RECORD]}, tmp_path)
+    assert "raised to `E-MANIFEST-ENTRY-MISMATCH`: no findings left, remove the record |" in report
+
+
+def test_errors_of_rules_nobody_accepted_stay_out_of_the_warnings_table(tmp_path):
+    output = RAISED_OUTPUT.replace("E-MANIFEST-ENTRY-MISMATCH]", "E-SCHEMA]")
+    report = quality_state.warnings_report(output, {}, tmp_path)
+    assert "E-SCHEMA" not in report
+
+
+def test_an_error_cannot_be_accepted(tmp_path):
+    with pytest.raises(ValueError, match="cannot be accepted"):
+        quality_state.accept_warnings(
+            {}, "E-MANIFEST-ENTRY-MISMATCH", RAISED_OUTPUT, tmp_path, reason="r", since="2026-10-12"
+        )
+
+
+def test_the_old_name_of_a_raised_rule_cannot_be_accepted_either(tmp_path):
+    with pytest.raises(ValueError, match="E-MANIFEST-ENTRY-MISMATCH"):
+        quality_state.accept_warnings(
+            {}, "W-MANIFEST-ENTRY-MISMATCH", RAISED_OUTPUT, tmp_path, reason="r", since="2026-10-12"
+        )
+
+
+def test_the_old_name_of_a_lowered_rule_is_recorded_under_its_new_one(tmp_path):
+    # Lowering renames too (E-X becomes W-X); the record carries the ID the
+    # findings carry, or it would compare against nothing from the next run on.
+    root = _lesson(tmp_path)
+    output = "RENAMED E-CLOZE-NO-CARRIER -> W-CLOZE-NO-CARRIER\n" + WARNINGS_OUTPUT
+    state = quality_state.accept_warnings({}, "E-CLOZE-NO-CARRIER", output, root, reason="r", since="2026-10-12")
+    [entry] = state["accepted_warnings"]
+    assert (entry["rule"], entry["ids"]) == ("W-CLOZE-NO-CARRIER", ["ex-abc"])
+
+
+def test_a_recorded_error_fails_loudly(tmp_path):
+    path = tmp_path / "state.json"
+    path.write_text(json.dumps({"accepted_warnings": [{**RAISED_RECORD, "rule": "E-MANIFEST-ENTRY-MISMATCH"}]}))
+    with pytest.raises(ValueError, match="cannot be accepted"):
+        quality_state.load_state(path)