From 8ccd778366b1e30411db6c1999ed989d52066d2f Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Sun, 17 May 2026 00:51:21 -0700 Subject: [PATCH] fix(canonicalization): predicate-based call lookup + skip double-canon on auto-resolve Two findings from Greptile review on PR #302: 1. tests/test_cli_v3.py:302 - The test asserted run_mock.call_args_list[0] was the main runner's invocation, but fanout.run_competitor_fanout submits main + competitors to a ThreadPoolExecutor and iterates with as_completed. With zero-latency mocks, thread scheduling determines which pipeline.run call lands first, so the competitor's call could take index [0] and flake CI. Replace [0] indexing with a predicate match on the canonicalized github_repos kwargs. 2. skills/last30days/scripts/last30days.py:662 - When auto_resolve returns github_repos, it has already run canonicalize_github_repos(cap=5) and ranked by relevance. The downstream block then re-canonicalized with cap=None, which can re-sort by topic-slug match and clobber the auto_resolve relevance order. Guard the second canonicalization with a repos_from_auto_resolve flag so it only fires for user-supplied --github-repo input. --- skills/last30days/scripts/last30days.py | 10 +++++++++- tests/test_cli_v3.py | 19 +++++++++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/skills/last30days/scripts/last30days.py b/skills/last30days/scripts/last30days.py index bdc37f3..37bab35 100644 --- a/skills/last30days/scripts/last30days.py +++ b/skills/last30days/scripts/last30days.py @@ -625,6 +625,7 @@ def main() -> int: # Auto-resolve: use web search to discover subreddits/handles before planning. # This is the engine-side equivalent of SKILL.md Steps 0.55/0.75 for platforms # without WebSearch (OpenClaw, Codex, raw CLI). + repos_from_auto_resolve = False if args.auto_resolve and not external_plan: from lib import resolve resolution = resolve.auto_resolve(topic, config) @@ -639,6 +640,9 @@ def main() -> int: sys.stderr.write(f"[AutoResolve] GitHub user: @{args.github_user}\n") if resolution.get("github_repos") and not args.github_repo: args.github_repo = ",".join(resolution["github_repos"]) + # auto_resolve already canonicalized via canonicalize_github_repos(cap=5); + # mark so we don't re-canonicalize below and clobber its relevance order. + repos_from_auto_resolve = True sys.stderr.write(f"[AutoResolve] GitHub repos: {args.github_repo}\n") if resolution.get("context"): # Inject context into external_plan metadata for the planner to use @@ -651,7 +655,11 @@ def main() -> int: github_user = args.github_user.lstrip("@").lower() if args.github_user else None github_repos = [r.strip() for r in args.github_repo.split(",") if r.strip() and "/" in r.strip()] if args.github_repo else None - if github_repos: + # Only canonicalize when repos came from a user-supplied --github-repo flag. + # When repos_from_auto_resolve is True, auto_resolve already ran + # canonicalize_github_repos(cap=5) and ranked by relevance; re-running here + # with cap=None can re-sort by topic-slug match and lose that ordering. + if github_repos and not repos_from_auto_resolve: from lib import resolve as resolve_lib original_github_repos = github_repos[:] github_repos = resolve_lib.canonicalize_github_repos(topic, github_repos, cap=None) diff --git a/tests/test_cli_v3.py b/tests/test_cli_v3.py index cac250d..153d6cc 100644 --- a/tests/test_cli_v3.py +++ b/tests/test_cli_v3.py @@ -289,12 +289,19 @@ class CliV3Tests(unittest.TestCase): with redirect_stdout(stdout), redirect_stderr(stderr): rc = cli.main() self.assertEqual(0, rc) - # In vs-mode the main_runner is the first pipeline.run call; sub-runs - # for competitor entities follow with their own per-entity github_repos. - kwargs = run_mock.call_args_list[0].kwargs - self.assertEqual( - ["openai/codex", "anthropics/claude-code"], - kwargs["github_repos"], + # In vs-mode main + competitors run in parallel via ThreadPoolExecutor, + # so the order of pipeline.run invocations is non-deterministic. Find + # the main runner's call by predicate on the canonicalized github_repos + # rather than by index. + expected_repos = ["openai/codex", "anthropics/claude-code"] + main_call = next( + (c for c in run_mock.call_args_list if c.kwargs.get("github_repos") == expected_repos), + None, + ) + self.assertIsNotNone( + main_call, + f"No pipeline.run call had github_repos={expected_repos}; " + f"saw {[c.kwargs.get('github_repos') for c in run_mock.call_args_list]}", ) self.assertIn("[GitHub] Canonicalized repos:", stderr.getvalue())