Skip to content

Commit 46b4737

Browse files
shenxianpengclaude
andcommitted
fix: print the failure reason once and keep the log tree intact
The job log showed the same sentence twice in a row under every failure: Error: Commit message should use imperative mood (...) value: feat: settle the report format ... Commit message should use imperative mood (...) The first line was an ::error workflow command carrying the first line of the error, the third was the listing printing the whole error underneath it. The annotation also broke the listing it sat inside. An ::error renders as a line of its own wherever it appears, so printing one between the ✖ scope line and its details split the indented tree apart. And its title= — the only place the rule ID was written — is shown in the annotations UI, never inline, so the visible line lost the rule ID entirely. Annotations are now emitted after every group, one per failure, with the scope in the message since the title cannot carry it. The listing keeps the reason, the rule ID, the value, the suggestion and the docs link, indented under the scope that failed. Passing scopes show the value that was checked, which the Markdown report already did. Also drops the "(0 failures)" that unparsable output rendered next to its ✖: there is no check list to count, and zero next to a cross reads as a contradiction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
1 parent a280b6d commit 46b4737

2 files changed

Lines changed: 102 additions & 20 deletions

File tree

‎main.py‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -367,38 +367,72 @@ def _grouped(results: list[ScopeResult]) -> list[tuple[str, list[ScopeResult]]]:
367367
return groups
368368

369369

370+
def _annotation_escape(text: str) -> str:
371+
"""Escape text for a workflow command payload.
372+
373+
A newline would end the command and leave the rest of the message as a
374+
stray log line, and a bare ``%`` can be read as the start of an escape.
375+
"""
376+
return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A")
377+
378+
370379
def render_step_log(results: list[ScopeResult]) -> None:
371-
"""Print results to the step log with folded groups and error annotations."""
380+
"""Print results to the step log, then emit one annotation per failure.
381+
382+
The two are separated deliberately. An ``::error`` command renders as a
383+
line of its own wherever it is printed, so emitting one inside the indented
384+
listing broke the tree apart, and its ``title=`` \u2014 which is what carries the
385+
rule ID \u2014 is only shown in the annotations UI, never inline. Printing the
386+
detail once in the listing and the annotations after all the groups keeps
387+
the log readable and still surfaces failures in the run summary and on the
388+
Files changed tab.
389+
"""
390+
annotations: list[tuple[str, str]] = []
391+
372392
for group_name, scopes in _grouped(results):
373393
print(f"::group::{group_name}")
374394
for scope in scopes:
395+
value = _scope_value(scope)
396+
suffix = f" ({value})" if value else ""
375397
if scope.status == "pass":
376-
print(f" \u2714 {scope.label}")
398+
print(f" \u2714 {scope.label}{suffix}")
377399
continue
378-
failures = scope.failures
379-
count = f" ({len(failures)} failure{'s' if len(failures) != 1 else ''})"
380-
print(f" \u2716 {scope.label}{count}")
381400
if scope.raw_text and not scope.checks:
382401
# Defensive fallback: commit-check produced unexpected output.
402+
print(f" \u2716 {scope.label}")
383403
for line in scope.raw_text.strip().splitlines():
384-
print(f" {line}")
404+
print(f" {line}")
405+
annotations.append(
406+
(f"commit-check: {scope.label}", "output could not be parsed")
407+
)
385408
continue
409+
failures = scope.failures
410+
count = f" ({len(failures)} failure{'s' if len(failures) != 1 else ''})"
411+
print(f" \u2716 {scope.label}{count}")
386412
for check in failures:
387-
title = _rule_label(check)
413+
label = _rule_label(check)
388414
error = check.get("error", "")
389-
first_line = error.splitlines()[0] if error else "check failed"
390-
print(f"::error title={title}::{first_line}")
415+
print(f" {label}")
391416
if check.get("value"):
392-
print(f" value: {check['value']}")
417+
print(f" value: {check['value']}")
393418
if error:
394-
print(f" {error}")
419+
for line in error.splitlines():
420+
print(f" {line}")
395421
if check.get("suggest"):
396-
print(f" Suggest: {check['suggest']}")
422+
print(f" Suggest: {check['suggest']}")
397423
if check.get("docs_url"):
398-
print(f" Docs: {check['docs_url']}")
424+
print(f" Docs: {check['docs_url']}")
425+
first_line = error.splitlines()[0] if error else "check failed"
426+
annotations.append((label, f"{scope.label}: {first_line}"))
399427
print("::endgroup::")
400428

401-
if all(scope.status == "pass" for scope in results):
429+
for title, message in annotations:
430+
print(
431+
f"::error title={_annotation_escape(title)}"
432+
f"::{_annotation_escape(message)}"
433+
)
434+
435+
if not annotations:
402436
print("\u2714 commit-check: all checks passed")
403437

404438

‎main_test.py‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -608,14 +608,58 @@ def test_failure_prints_group_and_error_annotation(self):
608608
self.assertIn("::group::Commit message", output)
609609
self.assertIn("::endgroup::", output)
610610
self.assertIn("✖ Commit 1/1 (1 failure)", output)
611-
self.assertIn(
612-
"::error title=CC001 message::The commit message should follow "
613-
"Conventional Commits.",
614-
output,
615-
)
611+
self.assertIn(" CC001 message", output)
616612
self.assertIn("value: bad message", output)
617613
self.assertIn("Suggest: Use <type>(<scope>): <description>", output)
618614
self.assertIn("Docs: https://commit-check.com/rules/#cc001", output)
615+
# The annotation names the scope, which the title alone cannot carry.
616+
self.assertIn(
617+
"::error title=CC001 message::Commit 1/1: The commit message should "
618+
"follow Conventional Commits.",
619+
output,
620+
)
621+
622+
def test_failure_reason_is_printed_once(self):
623+
"""The listing and the annotation must not both print the reason.
624+
625+
They used to: the ::error command carried the first line of the error
626+
and the listing printed the whole error underneath it, so the same
627+
sentence appeared twice in a row in the job log.
628+
"""
629+
output = self._run([fail_scope("Commit 1/1")])
630+
self.assertEqual(
631+
output.count("The commit message should follow Conventional Commits."),
632+
2, # once in the listing, once in the annotation after the groups
633+
)
634+
listing = output.split("::endgroup::")[0]
635+
self.assertEqual(
636+
listing.count("The commit message should follow Conventional Commits."), 1
637+
)
638+
639+
def test_annotations_come_after_every_group(self):
640+
"""An ::error inside a group breaks the indented listing apart."""
641+
output = self._run([fail_scope("Commit 1/1"), pass_scope("Branch")])
642+
self.assertLess(output.rindex("::endgroup::"), output.index("::error "))
643+
644+
def test_annotation_payload_is_escaped(self):
645+
scope = main.ScopeResult(
646+
label="Commit 1/1",
647+
checks=[
648+
make_check(
649+
"message",
650+
status="fail",
651+
error="first line\nsecond line with 100% certainty",
652+
)
653+
],
654+
)
655+
output = self._run([scope])
656+
annotation = [ln for ln in output.splitlines() if ln.startswith("::error")][0]
657+
self.assertNotIn("\n", annotation.removeprefix("::error "))
658+
self.assertIn("first line", annotation)
659+
660+
def test_pass_scopes_show_the_checked_value(self):
661+
output = self._run([pass_scope("Branch", value="feature/add-login")])
662+
self.assertIn("✔ Branch (feature/add-login)", output)
619663

620664
def test_groups_scopes_by_category(self):
621665
results = [
@@ -632,8 +676,12 @@ def test_groups_scopes_by_category(self):
632676
def test_raw_text_fallback_is_printed(self):
633677
scope = main.ScopeResult(label="Branch", raw_text="unexpected output")
634678
output = self._run([scope])
635-
self.assertIn("✖ Branch (0 failures)", output)
679+
# No "(0 failures)": there is no check list to count, and claiming zero
680+
# next to a ✖ reads as a contradiction.
681+
self.assertIn("✖ Branch", output)
682+
self.assertNotIn("0 failures", output)
636683
self.assertIn("unexpected output", output)
684+
self.assertIn("::error title=commit-check: Branch::", output)
637685

638686

639687
class TestRenderJobSummary(unittest.TestCase):

0 commit comments

Comments
 (0)