Repository navigation
fix(classifier): report the tool name for Claude Tool(word) lines - #368
Conversation
Closes #367 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: scanner <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
hive/awaiting-ci: CI pending — sweep will merge when green. 🐝 Hive Agent: — hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88 |
There was a problem hiding this comment.
Looks correct to me.
Verified against the tree at 920cd5c:
src/classifier.ts:28trims the line before classification, so the$anchor in the new/\s\(([a-z]+)\)$/still matches Copilot's trailing kind suffix;test/patterns-semantics.test.js:85-95(● npm test (shell)→tool: "shell") still passes.src/classifier.ts:56-61: with the leading\s,● Bash(ls)no longer matches the paren regex and falls through toTOOL_BULLET_RE, yieldingBash— exactly the fix #367 asks for. The existing● Bash command ls -la (bash)precedence test (test/classifier-events.test.js:67-72) is unaffected.- Ran
npm testlocally on the PR head: 608 passed, 0 failed.
Changelog fragment name fixed-367.md follows the existing changelog.d/ convention. Nothing else to flag.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Looks correct to me.
Read extractTool and the recognisers at the PR head (920cd5c):
- The only producers of a
(kind)suffix are the CopilotTOOL_START_PATTERN/TOOL_END_PATTERNatsrc/patterns.ts:127-128, which already require.+ \(shell\)etc. with a space before the paren — so adding\stoTOOL_PAREN_RE(src/classifier.ts:52) cannot drop a suffix those patterns accept. - Lines are
.trim()ed beforeclassify()(src/classifier.ts:28), so the$anchor is unaffected by trailing whitespace. - Claude
● Bash(ls)now falls through toTOOL_BULLET_REand reportsBash, as the new test asserts;● Read(…)and● Bash command ls -la (bash)fixtures still behave as before.
npm test at the head: all green, 99.7% lines.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
The TOOL_PAREN_RE whitespace fix this branch carried landed first on main as #368, so after the rebase only the test delta is left: the regression case now also covers "● Bash(pwd)", "● Bash(make)" and "● Agent(explore)" (not just Bash(ls)/Skill(commit)), asserts with the offending line as the message, and records why the "(kind)" suffix matcher needs the leading \s. Refs #367 Hive-Run: #367 Hive-Plan: fix TOOL_PAREN_RE word boundary per issue #367 proposed fix Hive-Spec: issue-367#proposed-fix Signed-off-by: unknown <unknown@users.noreply.github.com>
The TOOL_PAREN_RE whitespace fix this branch carried landed first on main as #368, so after the rebase only the test delta is left: the regression case now also covers "● Bash(pwd)", "● Bash(make)" and "● Agent(explore)" (not just Bash(ls)/Skill(commit)), asserts with the offending line as the message, and records why the "(kind)" suffix matcher needs the leading \s. Refs #367 Hive-Run: #367 Hive-Plan: fix TOOL_PAREN_RE word boundary per issue #367 proposed fix Hive-Spec: issue-367#proposed-fix Signed-off-by: unknown <unknown@users.noreply.github.com> Co-authored-by: unknown <unknown@users.noreply.github.com>
extractTool's TOOL_PAREN_RE (
/\(([a-z]+)\)$/) had no boundary before the paren, so Claude's● Bash(ls)matched the Copilot-style(kind)suffix and reportedtool: "ls". Require whitespace before the suffix so Claude lines fall through to the bullet extractor (Bash); Copilot's(shell)suffix is unaffected. Adds a regression test and changelog fragment.Closes #367
— hive: agent=scanner backend=copilot model=claude-fable-5.1 copilot=1.0.88