Skip to content

chore(summary): drop --fix-first and --held, which nothing reads - #55

Merged
ZhenghuaBao merged 1 commit into
mainfrom
chore/drop-dead-summary-flags
Sep 21, 2026
Merged

ZhenghuaBao merged 1 commit into
mainfrom
chore/drop-dead-summary-flags

Conversation

@ZhenghuaBao

@ZhenghuaBao ZhenghuaBao commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Orca-Code-Review — push 1

Severity Count
P0 0
P1 0
P2 0
P3 0

✅ no blocking findings

summary-comment.mjs parses two flags that nothing downstream reads.

Both survived the cascade that gave them meaning. --held marked a cheap pass withholding a strong review, and --fix-first let the blocking count follow the fix-first set on such a run — because block-on need not contain those severities, and "❌ 0 findings block merge" beside a held tier line contradicts itself.

There is no withholding and no tier line any more, so the count has followed block-on alone for some time. Meanwhile --held was still parsed into opts.held, and --fix-first was still parsed and validated against SEVERITIES, with neither value read anywhere after that.

fix-first the input is untouched

It stops the exhaustive loop early (action.yml), which is a different question from what this summary counts. The header now says so explicitly, so the shared name cannot argue its way back into the count later.

Safe to ship on its own

The argument loop has no else branch, so an unrecognised flag — and the value after it — are skipped one token at a time without shifting the flags that follow. The surviving block-on test now passes --fix-first P0 in the middle of the argv rather than at the end, which turns that property into a regression guard instead of an assumption.

Docs

skills/orca-review-action/references/inputs.md now scopes itself to this action. The hosted GitHub App runs a different review engine, so a setting of the same name can behave differently there — fix-first and exhaustive in particular — and the file says to answer App questions from the console rather than from these rows.

Verification

npm test: 558 pass / 9 fail before this change, and 558 pass / 9 fail after — byte-identical counts. The 9 are a pre-existing CRLF mismatch in the action.yml input assertions on a Windows checkout, unrelated to these files.

🤖 Generated with Claude Code

Both flags survived the cascade that gave them meaning. --held marked a
cheap pass withholding a strong review, and --fix-first let the blocking
count follow the fix-first set on such a run, because block-on need not
contain those severities. There is no withholding and no tier line any
more, so the count has followed block-on alone for some time — while the
two flags went on being parsed, and --fix-first went on being validated,
with neither value read by anything downstream.

fix-first REMAINS an action input. It stops the exhaustive loop early
(action.yml), which is a different question from what the summary counts;
the header now says so, so the name cannot argue its way back into this
count later.

Removing a flag the script no longer understands is safe to ship on its
own: the argument loop has no else branch, so an unrecognised flag and the
value after it are skipped one token at a time without shifting the flags
that follow. The surviving block-on test now passes --fix-first in the
MIDDLE of the argv to hold that property down.

Also scopes the action-inputs reference to this action: the hosted App runs
a different review engine, so fix-first and exhaustive can behave
differently there, and the file now says to answer App questions from the
console instead.

Test suite is unchanged by this: 558 pass / 9 fail before and after, the
failures being a pre-existing CRLF mismatch in the action.yml input tests
on a Windows checkout.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@ZhenghuaBao

Copy link
Copy Markdown
Contributor Author

@codex review

@ZhenghuaBao

Copy link
Copy Markdown
Contributor Author

@orcacode-review

@orcacode-review orcacode-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐳 OrcaCode Review

✅ No findings — nothing to flag in this PR. Great work!

OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 164 calls · 4M tokens · 92% cached

❤️ Share · Install OrcaCode Review

Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.

Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter

@ZhenghuaBao
ZhenghuaBao merged commit a49ffb5 into main Sep 21, 2026
2 checks passed
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