Skip to content

Commit 8f55aab

Browse files
shenxianpengclaude
andcommitted
chore: merge the remote branch's scope-warning fix
The remote claude/submit-patch-commit-check-42ac3i already carried 192d6f7 (report a warning even when its scope also fails), which is not in main and is not an ancestor of the fixes on this branch, so the push was rejected as a non-fast-forward. Merge it rather than rewrite either history. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018xYa7m3qup5wyN5MaXFgf6
2 parents 7158f3a + 192d6f7 commit 8f55aab

2 files changed

Lines changed: 98 additions & 26 deletions

File tree

‎main.py‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -536,24 +536,26 @@ def _render_scopes(scopes: list[ScopeResult], include_docs: bool) -> list[str]:
536536
value = _scope_value(scope)
537537
lines.append(f" ✔ {scope.label}{f' ({value})' if value else ''}")
538538
continue
539-
if scope.status == "warn":
540-
# Reported like a failure — same detail lines — but never a ✖:
541-
# a warned rule ran and found something, it just does not fail
542-
# the workflow, and the marker says so at a glance.
543-
warnings = scope.warnings
544-
count = f" ({len(warnings)} warning{'s' if len(warnings) != 1 else ''})"
545-
lines.append(f" ⚠ {scope.label}{count}")
546-
lines.extend(_render_findings(warnings, include_docs))
547-
continue
548539
if scope.raw_text and not scope.checks:
549540
# Defensive fallback: commit-check produced unexpected output.
550541
lines.append(f" ✖ {scope.label}")
551542
lines.extend(f" {ln}" for ln in scope.raw_text.strip().splitlines())
552543
continue
544+
# A scope's status names its worst outcome (fail beats warn), but the
545+
# two are not exclusive: CC2xx covers both branch and merge_base, so
546+
# one can fail while the other only warns. Render whichever of the
547+
# two lists is non-empty, rather than only the one the status names —
548+
# a warning on an otherwise-failing scope is still a finding to fix.
553549
failures = scope.failures
554-
count = f" ({len(failures)} failure{'s' if len(failures) != 1 else ''})"
555-
lines.append(f" ✖ {scope.label}{count}")
556-
lines.extend(_render_findings(failures, include_docs))
550+
if failures:
551+
count = f" ({len(failures)} failure{'s' if len(failures) != 1 else ''})"
552+
lines.append(f" ✖ {scope.label}{count}")
553+
lines.extend(_render_findings(failures, include_docs))
554+
warnings = scope.warnings
555+
if warnings:
556+
count = f" ({len(warnings)} warning{'s' if len(warnings) != 1 else ''})"
557+
lines.append(f" ⚠ {scope.label}{count}")
558+
lines.extend(_render_findings(warnings, include_docs))
557559
return lines
558560

559561

@@ -697,40 +699,48 @@ def _skip_count(results: list[ScopeResult]) -> int:
697699

698700

699701
def _warn_count(results: list[ScopeResult]) -> int:
700-
"""Number of scopes reported as warnings.
702+
"""Number of scopes that carry at least one warning.
701703
702-
Reported separately from the pass count, the same way skips are, so the
703-
headline can say how many findings were surfaced without claiming they
704-
failed anything.
704+
Counts by ``scope.warnings``, not ``scope.status == "warn"``: a scope
705+
whose overall status is "fail" (CC2xx covers both ``branch`` and
706+
``merge_base``, so one can fail while the other only warns) still has
707+
warnings to report, and this is the count the verdict and the table use
708+
to decide whether to show them.
705709
"""
706-
return sum(1 for scope in results if scope.status == "warn")
710+
return sum(1 for scope in results if scope.warnings)
707711

708712

709713
def _markdown_table(
710714
results: list[ScopeResult], status: str = "fail", header: str = "Failed checks"
711715
) -> str:
712716
"""Render the failure or warning table shared by summary and PR comment.
713717
714-
Only scopes of the given status appear, so a per-row result column would
715-
read the same symbol on every row and carry no information; the pass/fail
716-
picture for everything else lives in the details block.
718+
A scope appears when it has an entry of the requested kind \u2014 checked via
719+
``scope.failures`` / ``scope.warnings``, not ``scope.status`` \u2014 so a scope
720+
that both failed and warned gets a row in both tables. Filtering on the
721+
scope's single overall status would silently drop its warnings once a
722+
failure in the same scope outranked them.
717723
"""
718724
rows = [
719725
f"| Scope | Checked value | {header} |",
720726
"|---|---|---|",
721727
]
728+
is_raw_only_failure = (
729+
status == "fail"
730+
) # a raw-text scope has no checks to warn about
722731
for scope in results:
723-
# A skipped scope has no failed checks and no checked value, so it
724-
# contributed an entirely blank row \u2014 an empty accusation in a table
725-
# headed "Failed checks" \u2014 before this filter existed.
726-
if scope.status != status:
732+
entries = scope.failures if status == "fail" else scope.warnings
733+
raw_failure = is_raw_only_failure and scope.raw_text and not scope.checks
734+
# A skipped or clean-passing scope has no matching entries and is not
735+
# a raw-text failure, so it contributes no row \u2014 an empty accusation
736+
# in a table headed "Failed checks" or "Warnings" otherwise.
737+
if not entries and not raw_failure:
727738
continue
728739
value = _scope_value(scope)
729740
value_display = f"`{value}`" if value else "\u2014"
730-
if scope.raw_text and not scope.checks:
741+
if raw_failure:
731742
links = "_output could not be parsed \u2014 see details_"
732743
else:
733-
entries = scope.failures if status == "fail" else scope.warnings
734744
links = " \u00b7 ".join(_rule_markdown_link(check) for check in entries)
735745
rows.append(f"| {scope.label} | {value_display} | {links} |")
736746
return "\n".join(rows)

‎main_test.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,6 +1731,68 @@ def test_a_real_failure_outranks_a_warning_in_the_same_scope(self):
17311731
)
17321732
self.assertEqual(mixed.status, "fail")
17331733

1734+
def test_a_warning_survives_reporting_on_a_scope_that_also_fails(self):
1735+
"""The scope's status names its worst outcome, but both findings
1736+
still belong on every surface: the count, both tables, and the
1737+
details block. Filtering any of those on ``scope.status == "warn"``
1738+
drops this scope's warning the moment its sibling rule fails.
1739+
"""
1740+
mixed = main.ScopeResult(
1741+
label="Branch",
1742+
checks=[
1743+
make_check(
1744+
"branch",
1745+
status="warn",
1746+
rule_id="CC201",
1747+
value="jsmith/fix-x",
1748+
error="The branch should follow Conventional Branch.",
1749+
suggest="Use <type>/<description>",
1750+
docs_url="https://commit-check.com/rules/#cc201",
1751+
),
1752+
make_check(
1753+
"merge_base",
1754+
status="fail",
1755+
rule_id="CC202",
1756+
value="jsmith/fix-x",
1757+
error="Current branch is not rebased onto main.",
1758+
docs_url="https://commit-check.com/rules/#cc202",
1759+
),
1760+
],
1761+
)
1762+
self.assertEqual(main._warn_count([mixed]), 1)
1763+
1764+
body = main.render_report([mixed])
1765+
self.assertIn("❌ **1 of 1 check failed**, 1 warning", body)
1766+
self.assertIn("| Scope | Checked value | Failed checks |", body)
1767+
self.assertIn("[CC202 merge-base](https://commit-check.com/rules/#cc202)", body)
1768+
self.assertIn("| Scope | Checked value | Warnings |", body)
1769+
self.assertIn("[CC201 branch](https://commit-check.com/rules/#cc201)", body)
1770+
self.assertIn(" ✖ Branch (1 failure)", body)
1771+
self.assertIn(" CC202 merge-base", body)
1772+
self.assertIn(" ⚠ Branch (1 warning)", body)
1773+
self.assertIn(" CC201 branch", body)
1774+
1775+
def test_step_log_and_report_agree_on_a_mixed_scope(self):
1776+
"""The step log's ::warning annotation must not be the only surface
1777+
that shows this scope's warning — the report has to as well."""
1778+
mixed = main.ScopeResult(
1779+
label="Branch",
1780+
checks=[
1781+
make_check("branch", status="warn", rule_id="CC201", error="e1"),
1782+
make_check("merge_base", status="fail", rule_id="CC202", error="e2"),
1783+
],
1784+
)
1785+
buf = io.StringIO()
1786+
with patch("sys.stdout", buf):
1787+
main.render_step_log([mixed])
1788+
step_log = buf.getvalue()
1789+
self.assertIn("::warning title=CC201 branch::", step_log)
1790+
self.assertIn("::error title=CC202 merge-base::", step_log)
1791+
1792+
body = main.render_report([mixed])
1793+
self.assertIn("CC201 branch", body)
1794+
self.assertIn("CC202 merge-base", body)
1795+
17341796
def test_a_warning_outranks_a_skip_in_the_same_scope(self):
17351797
mixed = main.ScopeResult(
17361798
label="Author",

0 commit comments

Comments
 (0)