From 23416d97491c00fbbefa695999192f72a75cccf9 Mon Sep 17 00:00:00 2001 From: bymyself Date: Fri, 4 Sep 2026 15:41:02 +0000 Subject: [PATCH 1/2] fix(groom): bound finder exploration and spend --- .github/groom/finder.md | 4 +++- .github/groom/tests/test_finder_bounds.py | 27 +++++++++++++++++++++++ .github/workflows/groom.yml | 11 ++++++--- 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 .github/groom/tests/test_finder_bounds.py diff --git a/.github/groom/finder.md b/.github/groom/finder.md index a6ff0f70..0c3d9c59 100644 --- a/.github/groom/finder.md +++ b/.github/groom/finder.md @@ -2,7 +2,9 @@ You are a one-shot agent-work GROOM FINDER on the Mac Studio — phase 1 of 2. Y Find genuine, high-value refactor opportunities a thoughtful senior engineer would actually greenlight — NOT an exhaustive lint. Dimensions: (1) genuine duplication (same non-trivial logic ~15+ lines or a clear repeated shape across >=2 sites); (2) inconsistent patterns (one concept done N ways where converging helps — list the variants); (3) missing abstractions; (4) complexity hotspots; (5) dead/vestigial code. -HARD PRECISION BAR — this is the entire point: only things you'd stake your credibility on; ~6-12 findings MAX, ranked. EXPLICITLY AVOID premature abstraction (in Go especially, a little duplication beats the wrong abstraction; never DRY incidentally-similar-but-semantically-distinct code), bikeshedding, and anything linters/formatters already enforce. For EACH finding include a 'steelman-against' (the strongest reason NOT to do it) and DROP it if the steelman wins. +Work to a bounded inspection plan. After at most 60 inspection tool calls, stop exploring, write the best valid result supported by the evidence already gathered, and finish. Reserve enough time to write the result; broad repository coverage is NOT a completion requirement. If an inspection call is denied, do not retry it or seek a shell-command substitute — continue with the available Read, Glob, Grep, git log, git show, grep, cat, ls, head, tail, and wc tools. + +HARD PRECISION BAR — this is the entire point: only things you'd stake your credibility on; ~6-12 findings MAX, ranked. Fewer than 6 findings is valid, including zero, when that is all the bounded inspection supports. EXPLICITLY AVOID premature abstraction (in Go especially, a little duplication beats the wrong abstraction; never DRY incidentally-similar-but-semantically-distinct code), bikeshedding, and anything linters/formatters already enforce. For EACH finding include a 'steelman-against' (the strongest reason NOT to do it) and DROP it if the steelman wins. Write your result as VALID JSON to {{FINDER_OUT}}, EXACTLY this shape (JSON ONLY, no prose) — escape all string contents (quotes, backslashes, newlines): {"repo":"{{REPO}}","scope":"{{SCOPE_LABEL}}","findings":[{"title":"...","dimension":"...","sites":["file:line"],"evidence":"...","proposed":"...","value":"...","risk":"...","confidence":"high|med","steelman":"..."}]} diff --git a/.github/groom/tests/test_finder_bounds.py b/.github/groom/tests/test_finder_bounds.py new file mode 100644 index 00000000..595f282c --- /dev/null +++ b/.github/groom/tests/test_finder_bounds.py @@ -0,0 +1,27 @@ +import pathlib +import re +import unittest + + +ROOT = pathlib.Path(__file__).resolve().parents[3] +WORKFLOW = (ROOT / ".github/workflows/groom.yml").read_text(encoding="utf-8") +FINDER_BRIEF = (ROOT / ".github/groom/finder.md").read_text(encoding="utf-8") + + +class TestFinderBounds(unittest.TestCase): + def test_finder_cli_has_turn_and_dollar_caps(self): + finder_step = WORKFLOW.split("- name: Run finder", 1)[1].split( + "- name: Unlock the clone", 1 + )[0] + + self.assertRegex(finder_step, re.compile(r"--max-turns\s+100(?:\s|\\)")) + self.assertRegex(finder_step, re.compile(r"--max-budget-usd\s+8(?:\.0+)?(?:\s|\\)")) + + def test_brief_requires_an_early_result_and_no_denial_retries(self): + self.assertIn("After at most 60 inspection tool calls", FINDER_BRIEF) + self.assertIn("Fewer than 6 findings is valid", FINDER_BRIEF) + self.assertIn("If an inspection call is denied, do not retry it", FINDER_BRIEF) + + +if __name__ == "__main__": + unittest.main() diff --git a/.github/workflows/groom.yml b/.github/workflows/groom.yml index 56577c10..78cfd357 100644 --- a/.github/workflows/groom.yml +++ b/.github/workflows/groom.yml @@ -1380,8 +1380,12 @@ jobs: # auto-discovery (and hooks/plugins/keychain); auth stays on the # ANTHROPIC_API_KEY set above, the only credential this step has (--bare # never reads OAuth/keychain, so a missing key fails loudly, not silently). - # --max-turns 150: the whole-repo finder brief needs ~82 turns in a healthy - # environment (validated); the old 40 could not finish the brief at all. + # Two independent ceilings bound a runaway finder: 100 turns and $8. + # A healthy whole-repo run historically needed ~82 turns; the brief now + # reserves its final 40-turn margin for producing the result rather than + # treating repository coverage or a six-finding minimum as completion. + # The dollar ceiling is intentionally separate: turn cost varies with + # context size, so a turn cap alone allowed one failed run to spend $11.25. # `git grep` is deliberately NOT allowlisted: `--open-files-in-pager=` # executes an arbitrary command. The Grep tool covers content search. # `Edit(//)` instead of a bare `Write` is the STRUCTURAL close of @@ -1423,7 +1427,8 @@ jobs: GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=safe.directory GIT_CONFIG_VALUE_0='*' \ claude -p "$PROMPT" \ --model "$MODEL" \ - --max-turns 150 \ + --max-turns 100 \ + --max-budget-usd 8 \ --allowedTools "Read,Glob,Grep,Edit(//${FINDER_OUT#/}),Bash(git log:*),Bash(git show:*),Bash(grep:*),Bash(cat:*),Bash(ls:*),Bash(head:*),Bash(tail:*),Bash(wc:*)" \ --bare \ --setting-sources "" \ From 16f80459cb7c884bbdc8236c091b163227b48187 Mon Sep 17 00:00:00 2001 From: bymyself Date: Fri, 4 Sep 2026 15:51:13 +0000 Subject: [PATCH 2/2] fix(groom): reserve finder output budget Addresses https://github.com/Comfy-Org/github-workflows/pull/261#discussion_r3935631334 Addresses https://github.com/Comfy-Org/github-workflows/pull/261#discussion_r3935631345 Addresses https://github.com/Comfy-Org/github-workflows/pull/261#discussion_r3935631350 Addresses https://github.com/Comfy-Org/github-workflows/pull/261#discussion_r3935631359 --- .github/groom/finder.md | 2 +- .github/groom/tests/test_finder_bounds.py | 16 +++++++++++++--- .github/workflows/groom.yml | 7 +++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/.github/groom/finder.md b/.github/groom/finder.md index 0c3d9c59..f826d99f 100644 --- a/.github/groom/finder.md +++ b/.github/groom/finder.md @@ -2,7 +2,7 @@ You are a one-shot agent-work GROOM FINDER on the Mac Studio — phase 1 of 2. Y Find genuine, high-value refactor opportunities a thoughtful senior engineer would actually greenlight — NOT an exhaustive lint. Dimensions: (1) genuine duplication (same non-trivial logic ~15+ lines or a clear repeated shape across >=2 sites); (2) inconsistent patterns (one concept done N ways where converging helps — list the variants); (3) missing abstractions; (4) complexity hotspots; (5) dead/vestigial code. -Work to a bounded inspection plan. After at most 60 inspection tool calls, stop exploring, write the best valid result supported by the evidence already gathered, and finish. Reserve enough time to write the result; broad repository coverage is NOT a completion requirement. If an inspection call is denied, do not retry it or seek a shell-command substitute — continue with the available Read, Glob, Grep, git log, git show, grep, cat, ls, head, tail, and wc tools. +Work to a bounded inspection plan. The run has an $8 hard ceiling: treat $6 as the inspection budget and reserve the final $2 for synthesizing and writing the required JSON. Stop inspecting before the estimated inspection spend reaches $6; low remaining budget is a no-go for another inspection call. After at most 60 inspection tool calls, stop exploring, write the best valid result supported by the evidence already gathered, and finish. Reserve enough time to write the result; broad repository coverage is NOT a completion requirement. If an inspection call is denied, do not retry it or seek a shell-command substitute — continue with the available Read, Glob, Grep, git log, git show, grep, cat, ls, head, tail, and wc tools. HARD PRECISION BAR — this is the entire point: only things you'd stake your credibility on; ~6-12 findings MAX, ranked. Fewer than 6 findings is valid, including zero, when that is all the bounded inspection supports. EXPLICITLY AVOID premature abstraction (in Go especially, a little duplication beats the wrong abstraction; never DRY incidentally-similar-but-semantically-distinct code), bikeshedding, and anything linters/formatters already enforce. For EACH finding include a 'steelman-against' (the strongest reason NOT to do it) and DROP it if the steelman wins. diff --git a/.github/groom/tests/test_finder_bounds.py b/.github/groom/tests/test_finder_bounds.py index 595f282c..6f43aae3 100644 --- a/.github/groom/tests/test_finder_bounds.py +++ b/.github/groom/tests/test_finder_bounds.py @@ -13,14 +13,24 @@ def test_finder_cli_has_turn_and_dollar_caps(self): finder_step = WORKFLOW.split("- name: Run finder", 1)[1].split( "- name: Unlock the clone", 1 )[0] + claude_command = finder_step.split('claude -p "$PROMPT"', 1)[1].split( + "--output-format json", 1 + )[0] - self.assertRegex(finder_step, re.compile(r"--max-turns\s+100(?:\s|\\)")) - self.assertRegex(finder_step, re.compile(r"--max-budget-usd\s+8(?:\.0+)?(?:\s|\\)")) + self.assertRegex(claude_command, re.compile(r"--max-turns\s+100\s+\\")) + self.assertRegex(claude_command, re.compile(r"--max-budget-usd\s+8\s+\\")) def test_brief_requires_an_early_result_and_no_denial_retries(self): + self.assertIn("treat $6 as the inspection budget", FINDER_BRIEF) + self.assertIn("reserve the final $2", FINDER_BRIEF) + self.assertIn("low remaining budget is a no-go", FINDER_BRIEF) self.assertIn("After at most 60 inspection tool calls", FINDER_BRIEF) + self.assertIn("Reserve enough time to write the result", FINDER_BRIEF) self.assertIn("Fewer than 6 findings is valid", FINDER_BRIEF) - self.assertIn("If an inspection call is denied, do not retry it", FINDER_BRIEF) + self.assertIn( + "If an inspection call is denied, do not retry it or seek a shell-command substitute", + FINDER_BRIEF, + ) if __name__ == "__main__": diff --git a/.github/workflows/groom.yml b/.github/workflows/groom.yml index 78cfd357..470e0924 100644 --- a/.github/workflows/groom.yml +++ b/.github/workflows/groom.yml @@ -1381,11 +1381,10 @@ jobs: # ANTHROPIC_API_KEY set above, the only credential this step has (--bare # never reads OAuth/keychain, so a missing key fails loudly, not silently). # Two independent ceilings bound a runaway finder: 100 turns and $8. - # A healthy whole-repo run historically needed ~82 turns; the brief now - # reserves its final 40-turn margin for producing the result rather than - # treating repository coverage or a six-finding minimum as completion. + # The brief reserves both turn and dollar margin for producing the result + # rather than treating broad coverage or a finding minimum as completion. # The dollar ceiling is intentionally separate: turn cost varies with - # context size, so a turn cap alone allowed one failed run to spend $11.25. + # context size, so a turn cap alone is not a reliable spend boundary. # `git grep` is deliberately NOT allowlisted: `--open-files-in-pager=` # executes an arbitrary command. The Grep tool covers content search. # `Edit(//)` instead of a bare `Write` is the STRUCTURAL close of