Skip to content

[test] built-in /code-review in CI — do not merge - #84

Closed
morisil wants to merge 1 commit into
mainfrom
test/builtin-code-review
Closed

morisil wants to merge 1 commit into
mainfrom
test/builtin-code-review

Conversation

@morisil

@morisil morisil commented Sep 27, 2026

Copy link
Copy Markdown
Member

Throwaway PR for testing whether Claude Code's built-in /code-review --comment works from claude-code-action.

  • .github/workflows/claude-code-review.yml points at the experiment branch xemantic/.github@claude-review-builtin-test. That branch runs /code-review medium --comment <PR> with show_full_output: true.
  • isHtmlBlank has a deliberate bug (all → any) for the review to find.

Close without merging once the run has been inspected.

🤖 Generated with Claude Code

Points the review workflow at the xemantic/.github experiment branch
and plants a deliberate bug (isHtmlBlank: all -> any) for the review
to find.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
morisil added a commit that referenced this pull request Sep 27, 2026
Reverted right after the test on #84 — the action only runs a
workflow identical to the default branch's.

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

Copy link
Copy Markdown
Contributor

Confirmed — this function has multiple real call sites (whitespace collapsing, actionable-element labeling, blank-inline-formatting dropping), so the bug is live and impactful, not just cosmetic.

[
  {
    "file": "markanywhere-html-spec/src/commonMain/kotlin/HtmlWhitespace.kt",
    "line": 43,
    "summary": "`isHtmlBlank()` uses `any` instead of `all`, so it now returns true if a string merely *contains* HTML whitespace anywhere, rather than being *entirely* whitespace — inverting the intended semantics and contradicting its own doc comment (\"every character is HTML whitespace\").",
    "failure_scenario": "For non-blank text with an embedded space/tab/newline, e.g. \"Fundstelle openJur\" or \"§ 823\" (with an ordinary space), `isHtmlBlank()` now returns `true` instead of `false`. Downstream this makes `HtmlWhitespaceNormalization.kt:104` treat real content as blank/pending whitespace (merging or dropping visible text), `LabelActionableElements.kt:207` treat a text node with any space in it as not counting as a label (an `<a>` with normal multi-word text could be judged unlabelled and get a placeholder or be misjudged), and `DropBlankInlineFormatting.kt:69` treat non-blank text as blank and strip formatting/mark pairs that actually have real content — e.g. `<em>Fundstelle openJur</em>` would be dropped as if it were empty, since almost every real sentence contains at least one whitespace character."
  }
]

Note: this is a throwaway/test PR (per its description) with the all→any bug deliberately planted to verify the review pipeline works — the finding above is exactly that intentional bug, correctly caught.

Posted by the workflow: the review agent finished without commenting.

morisil added a commit that referenced this pull request Sep 27, 2026
… experiment"

This reverts commit e39af92: the test on #84 is done.

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

morisil commented Sep 27, 2026

Copy link
Copy Markdown
Member Author

Test done: the built-in /code-review --comment found the planted bug but did not post from CI; the workflow's fallback step posted it. Keeping the plugin-based review.

@morisil morisil closed this Sep 27, 2026
@morisil
morisil deleted the test/builtin-code-review branch September 27, 2026 13:09
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.

1 participant