Skip to content

feat(hooks): recognize Chinese keywords in the delegation nudge - #57

Open
leon7786 wants to merge 1 commit into
yuting0624:masterfrom
leon7786:feat/nudge-zh-keywords
Open

feat(hooks): recognize Chinese keywords in the delegation nudge#57
leon7786 wants to merge 1 commit into
yuting0624:masterfrom
leon7786:feat/nudge-zh-keywords

Conversation

@leon7786

Copy link
Copy Markdown

What

The nudge-delegation.sh UserPromptSubmit hook already nudges toward delegation when a prompt looks like bulk work — but only for English and Japanese phrases. This PR adds a conservative set of Chinese (ZH) volume/fan-out keywords so Chinese-language prompts get the same advisory nudge.

Keywords added

  • Bulk/scope: 批量, 全量, 所有文件, 全部文件, 整个项目, 整个仓库, 整个代码库, 遍历, 逐一
  • Migration: 迁移
  • Tests: 生成测试, 测试覆盖, 全套测试
  • Scaffolding: 脚手架
  • Research/search: 深度研究, 调研, 联网搜索, 网页搜索, 通读
  • Subagent / cross-model: 子代理, 子agent, 子 agent, 交叉验证, 交叉核对, 双模型, 第二意见, 复核
  • Review: 审查, 代码审查, 审阅, 校对
  • Parallel/bulk-gen: 并行, 批量生成

Notes

  • nocasematch is already on, so Latin-letter substrings (e.g. 子agent) match case-insensitively.
  • The space-bearing pattern *"子 agent"* is double-quoted — matching the existing *"all files"* convention — because an unquoted space would break the bash case word-splitting.
  • Matches are advisory only; the delegation decision stays with Claude, unchanged.
  • The header comment was updated (EN + JAEN + JA + ZH).

Verification

Tested locally via simulated UserPromptSubmit payloads:

  • ✅ Chinese bulk prompts trigger (迁移, 整个代码库, 交叉验证, 子代理, 审查 …)
  • ✅ Small / judgement-heavy Chinese prompts stay silent
  • ✅ Explicit agy-delegate prompts stay silent (no double-nudge)
  • ✅ Existing EN + JA matches unaffected (migrate every file, 一括, …)

🤖 Generated with Claude Code

Extend the UserPromptSubmit nudge heuristic to match Chinese
volume/fan-out phrases alongside the existing EN + JA sets, so
Chinese-language prompts for bulk work (migrations, exhaustive
tests, cross-model review, subagent fan-out) get the same advisory
nudge. Match is case-insensitive; space-bearing pattern is quoted
to keep the bash case syntax valid.

Co-Authored-By: Claude <noreply@anthropic.com>
@yuting0624 yuting0624 added documentation Improvements or additions to documentation claude-review Maintainer approval: run the Claude review on this (external) PR and removed documentation Improvements or additions to documentation claude-review Maintainer approval: run the Claude review on this (external) PR labels Aug 12, 2026
yuting0624 added a commit that referenced this pull request Aug 12, 2026
…IDC (#58)

* fix(ci): external review lost to its own concurrency group, then to OIDC

Two defects, both surfaced by #57 — the first fork PR to get far enough
to hit them.

1. A skippable run cancelled a live one. Two labels applied in the same
second produce two `labeled` events. The `claude-review` one started
`authorise`; the `documentation` one arrived two seconds later,
cancelled it, and then skipped itself because `github.event.label.name`
was not `claude-review`. Run 31556100775 shows `authorise cancelled`,
31556102147 `authorise skipped`, and the PR showed no review and no
reason. This is #42/#53 in quorum-review, in the one workflow that never
got the fix: `cancel-in-progress` is evaluated BEFORE any job condition.

A label never makes a running review obsolete — the head is unchanged.
Only a push does, and `false` queues rather than discards, so an
unrelated label waits its turn and then skips.

2. With that cleared, the action ran for the first time ever and the
OIDC exchange returned 401 Invalid OIDC token, three attempts. Not a
version skew: the 26 commits between this pin and the tag the internal
workflow resolves touch workflow_run support, not the exchange. The
internal workflow does the same exchange successfully on every run, so
the App is installed and the difference is the event.

Passing github_token explicitly skips the exchange. Upstream documents
the input as "optional if using GitHub App" and recommends passing it
for the analogous privileged context, so this is a supported path, not a
workaround — and it is the better one here. GITHUB_TOKEN is bounded by
this workflow's permissions block; an App installation token carries
whatever that App holds repository-wide. The narrower credential belongs
in the pull_request_target job.

Six assertions, each verified by mutation. The concurrency ones are
written separately from quorum's rather than shared: quorum keys on
github.event_name, this file is all pull_request_target and keys on the
action, and an assertion loose enough for both would stop guarding
either. One of them exists only because the extraction range differs —
this file puts permissions: before concurrency:, so quorum's range comes
back empty here and every assertion would have passed on nothing.

260 -> 262.

* docs: strip heredoc escaping artifacts from two test comments

Review nit: '"'"' survived into the comment text as literal characters
when these lines were written through a python heredoc. Harmless — they
are inside # comments — but they read as noise in the one place that
explains why the credential choice matters.
@yuting0624 yuting0624 added claude-review Maintainer approval: run the Claude review on this (external) PR and removed claude-review Maintainer approval: run the Claude review on this (external) PR labels Aug 12, 2026
Comment thread hooks/nudge-delegation.sh
*批量*|*全量*|*所有文件*|*全部文件*|*整个项目*|*整个仓库*|*整个代码库*| \
*迁移*|*生成测试*|*测试覆盖*|*全套测试*|*脚手架*|*深度研究*|*调研*|*联网搜索*|*网页搜索*|*遍历*|*逐一*| \
*子代理*|*子agent*|*"子 agent"*|*交叉验证*|*交叉核对*|*双模型*|*第二意见*|*复核*| \
*审查*|*代码审查*|*审阅*|*校对*|*并行*|*批量生成*|*通读*)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A few of these are broader than their EN/JA counterparts and could add false positives to an otherwise "deliberately conservative" list (per the header comment): 校对 (proofread), 复核 (double-check), 通读 (read through), and 审阅/审查 (review) can show up in small, non-bulk asks like "帮我校对这段话" or "复核一下这个函数". Worth double-checking whether these are meant to stay this broad — since the nudge is advisory-only and non-blocking this is low severity, but it's a bit inconsistent with the EN list, which has no generic "review" trigger at all.

@github-actions

Copy link
Copy Markdown

Thanks for this — nice, well-scoped addition, and the notes in the PR description (quoting the space-bearing pattern, keeping nocasematch in mind) show you'd already thought through the sharp edges here.

Security/scope: this only touches hooks/nudge-delegation.sh, which is advisory-only (fixed-string additionalContext, never echoes the prompt back, no network calls). It doesn't touch validate-delegate-bash.sh (the actual enforcement gate for the delegate subagent), tool permissions, or CI workflows. No concerns there.

Correctness: the added case patterns look syntactically sound — backslash-continued alternation, and the one space-bearing pattern ("子 agent") is correctly double-quoted to avoid word-splitting, matching the existing "all files" convention. nocasematch doesn't affect the multi-byte glyphs so no issue there.

Two small, non-blocking notes:

  1. Test coverage: tests/run-tests.sh has cases for the bulk EN prompt and the bulk JA prompt (nudge fires on bulk EN/JA prompt), but no equivalent ZH case was added even though the PR description mentions local verification. Since CI runs the suite on every PR, adding one case assertion (e.g. for 迁移) would keep this behavior locked in instead of only exercised by hand.
  2. Left an inline note on a few of the newer keywords (校对, 复核, 通读, 审阅/审查) that are broader than their EN/JA counterparts and could fire on smaller, non-bulk requests — low severity since the nudge is advisory and non-blocking, but worth a look given the "deliberately conservative" design goal stated in the file's header comment.

Nothing here blocks merging as far as I can tell — solid, low-risk contribution.

@yuting0624

Copy link
Copy Markdown
Owner

Thank you — this fills a real gap, and the notes in your description (the quoted "子 agent", the nocasematch reasoning) show you'd already looked at the sharp edges. I'd like to take it. One calibration issue first, with the measurements behind it.

The ZH set fires where the EN and JA sets stay quiet

I ran small, ordinary prompts through the hook with your patterns applied, alongside their English equivalents through the current hook:

Chinese prompt with this PR English equivalent today
遍历这个数组求和 NUDGE iterate over this array and sum it silent
这段代码用并行处理会不会更快 NUDGE would parallel processing be faster here silent
帮我做一次 k 折交叉验证 NUDGE do a k-fold cross-validation silent
帮我审查一下这个函数 NUDGE review this function silent
校对一下这句注释的措辞 NUDGE proofread this comment silent
调研一下这个库的 API NUDGE look into this library's API silent
复核一下这个数字对不对 NUDGE double-check this number silent
逐一确认这三个参数 NUDGE check these three params one by one silent

8 of 8 fire; 8 of 8 English equivalents don't. So a Chinese-speaking user gets nudged on work an English-speaking user doesn't — which is the opposite of what this PR is for, and it's what the header comment means by "deliberately conservative".

It's 10 patterns out of 33

调研 遍历 逐一 交叉验证 复核 审查 审阅 校对 并行 通读

Three of those are ordinary programming vocabulary — 遍历 (iterate), 并行 (parallel), 交叉验证 (also the ML term) — so they'll fire on small questions constantly. The other 22 are fine: I checked them against 一括-style prompts and they still catch every one.

Two are also redundant, since case matching is substring: 批量生成 is covered by 批量, and 代码审查 by 审查.

Suggested replacement

Dropping the 10 and keeping 代码审查 (which becomes meaningful once 审查 is gone):

  *一括*|*全ファイル*|*すべてのファイル*|*網羅*|*移行*|*大量*|*横断*|*リポジトリ全体*| \
  *批量*|*全量*|*所有文件*|*全部文件*|*整个项目*|*整个仓库*|*整个代码库*| \
  *迁移*|*生成测试*|*测试覆盖*|*全套测试*|*脚手架*|*深度研究*|*联网搜索*|*网页搜索*| \
  *子代理*|*子agent*|*"子 agent"*|*交叉核对*|*双模型*|*第二意见*|*代码审查*)

Measured on that set: 11 of 11 small prompts silent (the 8 above plus 这个正则为什么不匹配 / 把这个函数改成异步的 / 解释一下这段错误堆栈), and 8 of 8 bulk prompts still fire, including 用子代理并行跑这些迁移 and 生成测试覆盖整个项目.

Please add a test

tests/run-tests.sh has nudge fires on bulk EN prompt and nudge fires on bulk JA prompt, plus a nudge silent on a small prompt case. Two ZH lines in the same shape would lock this in — and the silent-on-small one is exactly the assertion that would have caught the above. Your description says you verified by hand; this just moves that into CI.

One design question, not a blocker

子代理 / 双模型 / 第二意见 / 代码审查 add a category that EN and JA don't have — cross-model verification and review. They're legitimate delegation signals for this plugin, so I'm inclined to keep them, but it leaves ZH broader than the other two languages. I'd rather add the EN/JA counterparts in a follow-up than drop them here. If you'd prefer to keep this PR a strict parallel of the existing sets, that's fine too — say which and I'll go with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude-review Maintainer approval: run the Claude review on this (external) PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants