Skip to content

feat(skills): add cross-issue regression sweep - #3065

Merged
ericksoa merged 14 commits into
mainfrom
feat/skill-cross-issue-sweep
May 7, 2026
Merged

feat(skills): add cross-issue regression sweep#3065
ericksoa merged 14 commits into
mainfrom
feat/skill-cross-issue-sweep

Conversation

@cjagwani

@cjagwani cjagwani commented May 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds nemoclaw-maintainer-cross-issue-sweep — scans other open issues to find ones a given PR may also fix or accidentally break. Outputs adjacent-fix opportunities (bundling intel) and contradiction risks (coordination flags) with file:line evidence.

Why

Reviewers focus on the issue a PR claims to fix. They almost never check whether the same diff affects other open issues. Surfacing "PR may also close #X, #Y" or "PR contradicts #Z" is decision-changing intel no existing skill provides.

5 clever signals

  1. Symbol-level fingerprinting — catches issues that mention a function name even in a different file
  2. Error-string fingerprinting — catches issues by user-pasted symptoms
  3. Two-direction relationship classification (adjacent + contradicting)
  4. Evidence-required LLM filter — must cite specific PR diff line + issue symptom; floors hallucination
  5. Reverse-link confidence boost — if candidate issue already mentions this PR, boost a tier (relationship is in someone's mental model)

Skill structure (mirrors v1)

.agents/skills/nemoclaw-maintainer-cross-issue-sweep/
├── SKILL.md
├── repo-policy.md
├── relationship-rules.md
├── checks/{fingerprint-extraction,relationship-judgment}.md
├── templates/report.md
├── validation/backtest.md
└── scripts/{extract-fingerprint,search-candidate-issues}.sh
    └── render-report.py

Composition with v1

The pr-comparator (PR #3052) calls this skill as a sub-step when comparing competing PRs. Adjacent-fix counts feed Tier 3 tiebreakers; contradicting hits factor into Tier 2 quality.

Defers (v3)

Sandboxed PR execution, static analyzer integration, ML symbol disambiguation.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a maintainer skill for automated cross-issue sweeps that surfaces adjacent fixes and contradicting risks.
    • Added end-to-end automation to extract PR fingerprints, discover candidate issues, classify relationships, apply boosts/filters, and render a consolidated Markdown report.
  • Documentation

    • Added step-by-step workflow, relationship classification rules, repo policy/configuration guidance, report template, fingerprint/search guidance, and a backtest validation guide.

Adds nemoclaw-maintainer-cross-issue-sweep with five 1%-clever signals:
symbol-level fingerprinting, error-string fingerprinting, two-direction
relationship classification, evidence-required LLM filter, and reverse-link
confidence boost.

Mirrors v1 pr-comparator structure: slim SKILL.md, references one level
deep, three utility scripts, repo-policy.md for cross-repo reuse.
@copy-pr-bot

copy-pr-bot Bot commented May 5, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new NemoClaw maintainer cross-issue sweep skill: manifest, workflow docs and rules, repo policy, templates, two shell scripts (fingerprint extraction, issue search), a Python report renderer, and a backtest validation guide.

Changes

Cross-Issue Sweep Workflow

Layer / File(s) Summary
Skill Definition & Workflow
.agents/skills/nemoclaw-maintainer-cross-issue-sweep/SKILL.md
New skill manifest and six-step workflow (fingerprint → search → classify → reverse-link boost → filter → render).
Fingerprint Spec / Policy Docs
.agents/skills/.../checks/fingerprint-extraction.md, .agents/skills/.../repo-policy.md
Documents fingerprint JSON shape (pr, files, symbols, error_strings, primary_issue) and repo policy (search caps, per-language symbol regexes, bot exclusions, confidence_floor).
Symbol/Error Extraction Implementation
.agents/skills/.../scripts/extract-fingerprint.sh
New Bash script: fetches PR diff via gh, extracts added lines, language-specific symbols and error-string tokens, determines primary linked issue, emits JSON fingerprint.
Candidate Discovery Implementation
.agents/skills/.../scripts/search-candidate-issues.sh
New Bash script: reads fingerprint JSON, queries GitHub issues by symbol, file path, and error string (per-dimension caps, dedupe, exclude primary), collects issue excerpts, emits candidates JSON.
Relationship Judgment Spec & Rules
.agents/skills/.../checks/relationship-judgment.md, .agents/skills/.../relationship-rules.md
LLM-facing judgment rules: inputs, prompt/evidence types (direct/by-omission/follow-on), classification labels (ADJACENT_FIX, CONTRADICTING, SAME_ISSUE_DIFF, UNRELATED), confidence levels, and reverse-link boost behavior.
Report Rendering & Template
.agents/skills/.../scripts/render-report.py, .agents/skills/.../templates/report.md
New Python renderer: reads classification JSON, validates structure, ranks by confidence (CONFIDENCE_RANK), partitions ADJACENT_FIX vs CONTRADICTING, and renders structured Markdown per template.
Validation / Backtest Guide
.agents/skills/.../validation/backtest.md
Backtest guide for retroactive evaluation: selecting historical cases, commands to run fingerprint/search, metrics (recall/precision), runtime targets, and failure modes to watch.

Sequence Diagram(s)

sequenceDiagram
  participant Maintainer
  participant GH as "GitHub (gh CLI)"
  participant Extract as "extract-fingerprint.sh"
  participant Search as "search-candidate-issues.sh"
  participant LLM as "Relationship Judgment"
  participant Render as "render-report.py"

  Maintainer->>GH: open PR / invoke scripts
  GH->>Extract: provide PR diff & metadata
  Extract->>Extract: compute files, symbols, error_strings, primary_issue
  Extract->>Search: emit fingerprint JSON
  Search->>GH: query issues by symbol/file/error string
  GH->>Search: return candidate issues
  Search->>LLM: provide candidate context per spec
  LLM->>LLM: classify candidates (ADJACENT_FIX/CONTRADICTING/OTHER)
  LLM->>Render: emit classifications JSON
  Render->>Maintainer: produce Markdown report
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped the diffs beneath moon's glow,

plucked symbols, errors, and links in tow,
nudged issues close where fixes hide,
flagged contradictions side by side,
then left a tidy report — neat and slow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(skills): add cross-issue regression sweep' is clear, concise, and directly describes the main change: adding a new skill for cross-issue regression detection.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/skill-cross-issue-sweep

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.agents/skills/nemoclaw-maintainer-cross-issue-sweep/SKILL.md (1)

1-121: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add maintainer skills to docs/ sources or explicitly exclude them from autogeneration.

The nemoclaw-maintainer-cross-issue-sweep skill directory and other maintainer skills were added directly to .agents/skills/ without corresponding sources in docs/. While docs-to-skills.py currently generates only user skills (via --prefix nemoclaw-user), there is no explicit exclusion protecting maintainer skills from accidental overwriting if the script is reconfigured. Either:

  1. Create docs/ source files for maintainer skills and regenerate with --prefix nemoclaw-maintainer, or
  2. Add an explicit exclusion pattern for nemoclaw-maintainer-* to docs-to-skills.py with a documented note that these are hand-maintained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/nemoclaw-maintainer-cross-issue-sweep/SKILL.md around lines 1
- 121, The maintainer skill files under .agents/skills (e.g.,
nemoclaw-maintainer-cross-issue-sweep/SKILL.md) were added without corresponding
docs sources and risk being overwritten by docs-to-skills.py; fix by either (A)
adding a docs/ source for this skill and regenerating with the --prefix
nemoclaw-maintainer flag so the skill is generated from docs, or (B) updating
docs-to-skills.py to explicitly exclude the nemoclaw-maintainer-* prefix (add an
exclusion pattern and a brief comment explaining these are hand-maintained) so
maintainer skills remain untouched; modify the SKILL.md placement or the
exclusion in docs-to-skills.py accordingly.
🧹 Nitpick comments (1)
.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh (1)

42-49: ⚡ Quick win

Go method receivers aren't captured.

The Go pattern 'func [A-Z][A-Za-z0-9_]*' only matches standalone functions. Methods with receivers like func (r *Repo) GetIssue() won't be captured because (r *Repo) appears between func and the name.

The documented regex in repo-policy.md handles this with (?:\([^)]*\)\s+)?, but that syntax requires PCRE (grep -P).

♻️ Possible fix using grep -P (if available)
-  printf '%s' "$1" | grep -oE 'func [A-Z][A-Za-z0-9_]*' | awk '{print $2}'
+  printf '%s' "$1" | grep -oP 'func\s+(?:\([^)]*\)\s+)?([A-Z][A-Za-z0-9_]*)' | grep -oE '[A-Z][A-Za-z0-9_]+$'

Alternatively, use a two-pass approach with extended regex:

# Match both standalone funcs and methods with receivers
printf '%s' "$1" | grep -oE 'func (\([^)]+\) )?[A-Z][A-Za-z0-9_]*' | grep -oE '[A-Z][A-Za-z0-9_]+$'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh
around lines 42 - 49, In extract_symbols update the Go "func" extraction so
method receivers are allowed: replace the current single-word match for func
names with a two-step regex that first matches "func" optionally followed by a
parenthesized receiver and then the exported name, then extract the final token
(the function name) — i.e., change the grep/awk pipeline inside extract_symbols
that handles 'func ...' to a pattern that accepts an optional "(...)" receiver
before the name and then prints the name (or, if grep -P is available, use the
repo-policy.md PCRE with '(?:\([^)]*\)\s+)?' to capture the name).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh:
- Around line 29-33: The jq filter assigned to the variable files incorrectly
applies | not only to the last endswith(...) check; update the filter in the
files=... command (the gh pr view invocation) so the three endswith(...) checks
are grouped and negated together (e.g. wrap the or sequence in parentheses or
use not(...) around the combined predicate) to ensure package-lock.json,
yarn.lock and pnpm-lock.yaml are all excluded as intended.

In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py:
- Around line 66-78: The code assumes spec has expected shapes and indexes
directly (pr, pr_title, classifications) which can raise exceptions for
malformed JSON; before using spec["pr"] and the comprehensions that build
adjacent and contradicting, validate and coerce the fields: ensure
spec.get("pr") exists (or handle missing), coerce pr_title =
str(spec.get("pr_title","")), and set classifications =
spec.get("classifications") if isinstance(..., list) else [] ; when building
adjacent and contradicting filter items with isinstance(c, dict) and use c.get
safely and default confidence/class to avoid KeyError/AttributeError; wrap
parsing in a small validation block and raise or log a controlled error if
required.

In @.agents/skills/nemoclaw-maintainer-cross-issue-sweep/validation/backtest.md:
- Around line 18-20: The gh search prs invocation uses multiple positional
arguments for the query which is invalid; replace the separate tokens with a
single query string by changing the command query argument used in the shown gh
search prs call to combine terms with OR and the in:body qualifier (e.g., use a
single quoted query like "supersedes OR alternative in:body") and keep the rest
of the flags (--repo, --merged, --limit 30) unchanged.

---

Outside diff comments:
In @.agents/skills/nemoclaw-maintainer-cross-issue-sweep/SKILL.md:
- Around line 1-121: The maintainer skill files under .agents/skills (e.g.,
nemoclaw-maintainer-cross-issue-sweep/SKILL.md) were added without corresponding
docs sources and risk being overwritten by docs-to-skills.py; fix by either (A)
adding a docs/ source for this skill and regenerating with the --prefix
nemoclaw-maintainer flag so the skill is generated from docs, or (B) updating
docs-to-skills.py to explicitly exclude the nemoclaw-maintainer-* prefix (add an
exclusion pattern and a brief comment explaining these are hand-maintained) so
maintainer skills remain untouched; modify the SKILL.md placement or the
exclusion in docs-to-skills.py accordingly.

---

Nitpick comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh:
- Around line 42-49: In extract_symbols update the Go "func" extraction so
method receivers are allowed: replace the current single-word match for func
names with a two-step regex that first matches "func" optionally followed by a
parenthesized receiver and then the exported name, then extract the final token
(the function name) — i.e., change the grep/awk pipeline inside extract_symbols
that handles 'func ...' to a pattern that accepts an optional "(...)" receiver
before the name and then prints the name (or, if grep -P is available, use the
repo-policy.md PCRE with '(?:\([^)]*\)\s+)?' to capture the name).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4fcc9e53-84e1-4224-9def-73c6d9730335

📥 Commits

Reviewing files that changed from the base of the PR and between f40c25e and 22e0039.

📒 Files selected for processing (10)
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/SKILL.md
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/checks/fingerprint-extraction.md
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/checks/relationship-judgment.md
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/relationship-rules.md
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/repo-policy.md
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/search-candidate-issues.sh
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/templates/report.md
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/validation/backtest.md

cjagwani added 2 commits May 5, 2026 15:32
Stress-test against 5 historical NemoClaw cases surfaced two real spec
gaps that produced incorrect classifications:

1. Evidence-required filter was too strict for partial-fix detection.
   The LLM couldn't cite "PR diff line" when the gap is what the PR
   did NOT touch. Extended evidence to three citation shapes:
   - DIRECT (original): cite specific PR line + issue symptom
   - BY-OMISSION: cite PR scope + issue symptom showing same bug class
     in different instances PR did not touch (catches partial-fix)
   - FOLLOW-ON: cite the symbol/file PR introduced + issue's request
     to harden the same code (catches "PR introduced X, now harden X")

2. ADJACENT_FIX wording assumed the issue was already broken and the
   PR resolves it. Reframed to also include "PR opens a clear follow-on
   path on the same code the PR just touched" — covers the common
   pattern where an issue requests hardening of code a PR just
   introduced.

Test cases that drove these patches:
- #2700#2762 (partial-fix, was failing — now passes via by-omission)
- #2696#2875 (follow-on hardening, was ambiguous — now explicit)
- extract-fingerprint.sh: jq operator precedence — wrap endswith chain
  in parens so 'or | not' applies to the whole disjunction (CR major)
- render-report.py: validate spec shape before indexing — check root is
  dict, 'pr' key present, classifications is list (CR major)
- backtest.md: gh search prs invalid syntax — combine query terms with
  OR in a single quoted string instead of multiple positional args (CR minor)
@cjagwani cjagwani self-assigned this May 5, 2026
@cjagwani
cjagwani requested a review from cv May 5, 2026 22:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (1)
.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py (1)

46-56: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard each classification entry before filtering/formatting.

Line 82/86 assumes every item is a dict (c.get(...)), and Line 53 assumes required keys exist. Malformed entries still cause uncaught exceptions.

Proposed fix
+    normalized_classifications: list[dict[str, Any]] = []
+    for i, c in enumerate(classifications):
+        if not isinstance(c, dict):
+            print(f"Invalid spec: classifications[{i}] must be a JSON object", file=sys.stderr)
+            return 64
+        normalized_classifications.append(c)
+
     adjacent = sorted(
-        [c for c in classifications if c.get("class") == "ADJACENT_FIX"],
+        [c for c in normalized_classifications if c.get("class") == "ADJACENT_FIX"],
         key=lambda c: -CONFIDENCE_RANK.get(c.get("confidence", "low"), 0),
     )
     contradicting = sorted(
-        [c for c in classifications if c.get("class") == "CONTRADICTING"],
+        [c for c in normalized_classifications if c.get("class") == "CONTRADICTING"],
         key=lambda c: -CONFIDENCE_RANK.get(c.get("confidence", "low"), 0),
     )

Also applies to: 81-87, 101-112

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py
around lines 46 - 56, The _format_entry function assumes its input c is a dict
with keys like 'issue_number', 'confidence', and an 'evidence' dict; guard
against malformed entries by first verifying isinstance(c, dict) and that
required keys exist (or else return a safe placeholder/skip), ensure evidence is
a dict before accessing evidence.get(...), and coerce values to strings when
interpolating (e.g., use str(c.get('issue_number','?')) and
str(c.get('confidence','?'))); apply the same defensive checks to other
formatting blocks referenced (the blocks around lines 81-87 and 101-112) so they
validate inputs, provide sensible defaults, or skip malformed records instead of
raising.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh:
- Around line 24-26: The current conditional around setting repo_args allows
calls like "extract-fingerprint.sh 123 --repo" to treat the missing repo value
as absent; update the check for the --repo flag so it validates that the next
positional parameter is present and is not another flag (e.g., ${2:-} is
non-empty and does not begin with '-') and if that validation fails, print a
clear usage/error message and exit non-zero; change the block that sets
repo_args=(--repo "$2") (and any surrounding logic that relies on $2) to perform
this validation and fail fast when --repo lacks a value.
- Line 48: The current pipeline in extract-fingerprint.sh that uses grep -oE
followed by awk to extract exported symbols misidentifies "export default" forms
because it captures the keyword rather than the identifier; replace that
pipeline (the printf | grep -oE ... | awk step) with a single regex-based
extraction that matches an optional "default", an optional keyword
(function/class/const/let/var), and then captures the actual identifier,
emitting only that capture (use a tool that supports capture-group printing such
as sed -nE or grep -oP) so only the symbol name is printed and only one match
succeeds for each input.
- Around line 42-49: The extract_symbols() and related pipelines can abort under
set -euo pipefail because grep returns exit code 1 on no matches; update every
grep pipeline inside extract_symbols(), the symbols assignment pipeline, and
extract_error_strings() to append "|| true" after the grep so the pipeline never
fails on empty input, and move the keyword exclusion filter currently
implemented with "grep -vE" into an awk-based filter (e.g., use awk to skip
keywords) so it also handles empty input without producing a non-zero exit code;
locate these changes in the extract_symbols function, the symbols pipeline, and
extract_error_strings to apply the fixes.
- Line 40: The current assignment to added_lines uses grep which returns exit
code 1 on no matches and causes the script to exit under set -euo pipefail;
update the pipeline that produces added_lines (the variable using "$diff") to
avoid grep and instead use sed to safely extract added lines without failing
(e.g. replace the printf ... | grep -E '^\+[^+]' | sed 's/^+//' pipeline with
printf '%s' "$diff" | sed -n 's/^+//p' so added_lines becomes an empty string
when there are no added lines rather than causing the script to exit).

In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py:
- Around line 79-99: The code assumes `suppressed` is a dict and calls
`suppressed.get(...)`, which will raise if `suppressed` is not a mapping; update
the handling of `suppressed` (the variable read from `spec.get("suppressed",
{})`) to validate and normalize its shape before use (e.g., if not an instance
of dict, replace with an empty dict or coerce expected keys to defaults) so
later calls like `suppressed.get("unrelated", 0)` and
`suppressed.get("same_issue_diff", 0)` are safe; apply the same normalization
wherever `suppressed` is read/used later in the script (the subsequent block
around the other usage noted).

---

Duplicate comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py:
- Around line 46-56: The _format_entry function assumes its input c is a dict
with keys like 'issue_number', 'confidence', and an 'evidence' dict; guard
against malformed entries by first verifying isinstance(c, dict) and that
required keys exist (or else return a safe placeholder/skip), ensure evidence is
a dict before accessing evidence.get(...), and coerce values to strings when
interpolating (e.g., use str(c.get('issue_number','?')) and
str(c.get('confidence','?'))); apply the same defensive checks to other
formatting blocks referenced (the blocks around lines 81-87 and 101-112) so they
validate inputs, provide sensible defaults, or skip malformed records instead of
raising.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e1414f0b-17e3-4cc2-8506-d0d0faa37af0

📥 Commits

Reviewing files that changed from the base of the PR and between 7d71cc3 and 4d3aee2.

📒 Files selected for processing (3)
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/validation/backtest.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/validation/backtest.md

@ericksoa ericksoa added v0.0.36 and removed v0.0.35 labels May 6, 2026

@ericksoa ericksoa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this skill. I found several blockers in the current head that make the feature fail on valid inputs:

  • extract-fingerprint.sh does not produce JSON on the documented path: the no---repo invocation fails with repo_args[@]: unbound variable, and extract-fingerprint.sh 3065 --repo NVIDIA/NemoClaw exits 1. The root cause is unguarded grep pipelines under set -euo pipefail for added lines, symbol/error extraction, and primary issue parsing.
  • Symbol extraction is materially wrong: export default function Foo() emits function instead of Foo, and Go receiver methods like func (r *Repo) GetIssue() are missed.
  • search-candidate-issues.sh exits non-zero on an empty valid fingerprint instead of returning {"candidates":[]}, and missing arrays crash jq. Empty or missing dimensions should be normal for small/doc-only PRs.
  • render-report.py still crashes on incomplete classifier output such as [null] classifications, entries missing issue_number/confidence, or non-dict suppressed.
  • validation/backtest.md uses gh search prs --in:body, which current gh rejects as an unknown flag.

Please fix these paths and add small regression/smoke coverage for empty/no-match inputs, malformed classifier output, export default, Go receiver methods, and the documented gh search commands before this ships.

cjagwani and others added 3 commits May 6, 2026 09:01
- extract-fingerprint.sh: fail fast when --repo has no value, swap grep|sed
  for sed -n on diff parsing (avoids pipefail abort on deletion-only diffs),
  add `|| true` to symbol/error-string greps, move keyword-exclusion filter
  to awk so empty input doesn't trip pipefail, split export extraction so
  `export default function foo` resolves to `foo` not `function`.
- render-report.py: validate `suppressed` is a dict before .get() use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- extract-fingerprint.sh: guard "${repo_args[@]}" so empty array doesn't
  trigger `unbound variable` under set -u; add `|| true` to primary_issue
  grep pipeline so PRs without `closes #N` don't exit 1; recognize Go
  receiver methods (`func (r *Repo) GetIssue` → `GetIssue`).
- search-candidate-issues.sh: jq `// []` defaults so missing
  symbols/files/error_strings dimensions don't crash; `|| true` on the
  primary-issue exclusion grep so all-primary results still emit
  `{"candidates": []}`.
- render-report.py: skip null/missing-key classifications instead of
  raising AttributeError; harden _format_entry against missing
  issue_number/confidence/evidence.
- validation/backtest.md: drop invalid `--in:body` flag; combine into the
  query string the way `gh search prs` actually accepts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cjagwani
cjagwani requested a review from ericksoa May 6, 2026 19:59
@cjagwani

cjagwani commented May 6, 2026

Copy link
Copy Markdown
Collaborator Author

@ericksoa thanks — fixed all five in b429177. tested each repro path locally before pushing. ready for re-review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh (1)

24-30: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden --repo parsing to reject flag-like values.

Line 25 only checks emptiness, so extract-fingerprint.sh 123 --repo --foo is accepted and later degrades into empty/fallback output. Treat missing/flag-like repo values as invalid input and exit 64.

Suggested fix
 if [ "${1:-}" = "--repo" ]; then
-  if [ -z "${2:-}" ]; then
+  if [ -z "${2:-}" ] || [[ "${2:-}" == -* ]]; then
     echo "Usage: $0 <pr-number> [--repo OWNER/REPO]" >&2
     exit 64
   fi
   repo_args=(--repo "$2")
+  shift 2
 fi
#!/usr/bin/env bash
set -euo pipefail

# Expected: both invocations should fail with usage (exit 64), not run with implicit defaults.
bash .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh 123 --repo || true
bash .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh 123 --repo --foo || true
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh
around lines 24 - 30, The --repo parsing in extract-fingerprint.sh currently
only checks for emptiness and will accept flag-like values (e.g., "--foo");
update the validation in the block that sets repo_args so that after detecting
"${1:-}" = "--repo" you also check that "${2:-}" is non-empty and does not start
with a hyphen (e.g., test [[ "${2:-}" == -* ]] or use case) and if it is invalid
print the usage message and exit 64; ensure repo_args=(--repo "$2") is only
assigned when the value passes this additional validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh:
- Around line 21-22: The script captures the PR into variable pr but doesn't
validate it, allowing non-numeric values to produce malformed JSON; add a
numeric check right after pr="$1" (and shift) that verifies pr matches a
digits-only regex (e.g., ^[0-9]+$), and if not, print an error to stderr and
exit with a non-zero status so the script fails fast and never emits `"pr": $pr`
with invalid content; ensure you keep pr as the numeric string (or convert to an
integer) so the JSON insertion remains a valid number.
- Around line 33-40: The current assignments to files and diff hide gh failures
by falling back to empty values; change the logic so gh pr view and gh pr diff
failures are detected and the script exits with an error instead of continuing.
Specifically, after running the gh pr view command used to populate the files
variable and the gh pr diff command used to populate diff, check the commands'
exit status and, on non‑zero, print a clear error message including which gh
operation failed (reference the gh pr view -> files assignment and gh pr diff ->
diff assignment) and exit with a non‑zero code; remove the silent "||
files='[]'" and "|| diff=''" fallbacks so authentication/repo/PR errors fail
fast.

In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/search-candidate-issues.sh:
- Around line 25-29: The script always autodetects the repo via gh repo view but
advertises a --repo flag; update the argument parsing at the top of
search-candidate-issues.sh to recognize an optional --repo OWNER/REPO parameter
and set the repo variable from it (prefer the provided value over
autodetection), e.g., parse $@ for --repo and assign to repo if present, then
only call gh repo view --json nameWithOwner when repo is empty, and keep the
existing error message/exit behavior if both methods fail; adjust any downstream
uses of the repo variable (repo) accordingly.

---

Duplicate comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh:
- Around line 24-30: The --repo parsing in extract-fingerprint.sh currently only
checks for emptiness and will accept flag-like values (e.g., "--foo"); update
the validation in the block that sets repo_args so that after detecting "${1:-}"
= "--repo" you also check that "${2:-}" is non-empty and does not start with a
hyphen (e.g., test [[ "${2:-}" == -* ]] or use case) and if it is invalid print
the usage message and exit 64; ensure repo_args=(--repo "$2") is only assigned
when the value passes this additional validation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 90b4ef88-e253-438f-9ed9-d446f58aad20

📥 Commits

Reviewing files that changed from the base of the PR and between 4d3aee2 and c369d78.

📒 Files selected for processing (4)
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/render-report.py
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/search-candidate-issues.sh
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/validation/backtest.md

cjagwani and others added 2 commits May 6, 2026 13:14
- extract-fingerprint.sh: validate <pr-number> is numeric (prevents
  malformed JSON like `"pr": abc,`); replace silent `|| files='[]'` /
  `|| diff=''` / `|| echo ''` fallbacks with explicit exit-65 on gh
  failures so auth/repo/PR-not-found doesn't masquerade as "no candidates."
- search-candidate-issues.sh: implement the `--repo OWNER/REPO` flag the
  error message advertises so the script works outside a checked-out repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
@.agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh:
- Around line 65-75: In extract_symbols() add two missing regex patterns: (1)
capture exported Go type names by adding a grep/awk line for "type
[A-Z][A-Za-z0-9_]*" (place it near the existing Go function patterns such as the
'func [A-Z][A-Za-z0-9_]*' and receiver-method 'func \([^)]+\)
[A-Z][A-Za-z0-9_]*' lines) so exported Go types are emitted, and (2) capture
POSIX-style shell function declarations by adding a sed pattern that matches
leading "name() { ..." (place it alongside the other shell/js export/const
patterns such as the 'export (function|class|const|let|var)' line) so standalone
shell functions are extracted; ensure both commands pipe to awk/sed to print
only the symbol and end with "|| true" to preserve original behavior.
- Around line 87-91: The extract_error_strings function currently only captures
'throw new Error("...")', 'console.error("...")', and generic quoted error-like
strings; update extract_error_strings to also capture (1) throw Error("...") (no
"new"), (2) Python f-strings printed via print(f"...") or standalone f-strings
containing error keywords, and (3) flag/option tokens like --no-color or
--verbose; add additional grep|sed pipeline lines (or extend existing regexes)
to match those patterns while preserving the existing fallback "|| true"
behavior and use the same output normalization approach used for throw new Error
/ console.error so callers of extract_error_strings (function name:
extract_error_strings) continue to receive one token per line.
- Around line 105-108: The primary_issue extraction only matches verbs like
"closes/fixes/resolves" and must also support the "Linked Issue: `#N`" block;
update the extraction logic that computes primary_issue (the grep/printf
pipeline) to also search for patterns like "Linked Issue\s*:\s*#N"
(case-insensitive) and prefer that match or fall back to the existing
verbs-based match—ensure the pipeline extracts just the digits (/[0-9]+/) from
either match and still uses head -n 1 to pick the first found ID.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9cf9c2f6-8b8a-4d3f-af41-c704dee5f383

📥 Commits

Reviewing files that changed from the base of the PR and between c369d78 and b8f21e8.

📒 Files selected for processing (2)
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/extract-fingerprint.sh
  • .agents/skills/nemoclaw-maintainer-cross-issue-sweep/scripts/search-candidate-issues.sh

Bring extract-fingerprint.sh in line with checks/fingerprint-extraction.md:

- Symbols: add Go `type ExportedName` and POSIX shell `name() {` patterns.
- Error strings: add `throw Error("...")` (no `new`), Python `print(f"...")`
  with error-shape keywords, and flag/option tokens like `--no-color`.
- Primary issue: support `Linked Issue: #N` block in addition to
  closes/fixes/resolves. Refactored the two-grep pipeline so both regexes
  read the body independently — the previous `body | { grep1; grep2; }`
  form failed because grep2 inherited an empty stdin after grep1 drained it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@cjagwani

cjagwani commented May 6, 2026

Copy link
Copy Markdown
Collaborator Author

@ericksoa all five blockers from your review are fixed in b42917785 and follow-on commits — verified the repro paths locally before pushing:

  • extract-fingerprint.sh 3065 (no --repo) → emits valid JSON, exit 0
  • extract-fingerprint.sh 3065 --repo NVIDIA/NemoClaw → emits valid JSON, exit 0
  • func (r *Repo) GetIssue()GetIssue extracted (Go receiver methods)
  • export default function Foo()Foo extracted
  • empty fingerprint → search-candidate-issues.sh returns {"candidates": []} cleanly
  • render-report.py survives [null] classifications + missing keys
  • validation/backtest.md uses gh search prs query syntax

since then I've also closed all CodeRabbit threads (3 rounds total) and added Linked-Issue parsing, numeric-PR validation, and --repo flag support on search-candidate-issues.sh.

ready for a re-review whenever.

@ericksoa ericksoa added v0.0.37 and removed v0.0.36 labels May 7, 2026

@ericksoa ericksoa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Current head looks good. I rechecked it against current main, verified the older CodeRabbit/review blockers are resolved, and ran focused smoke coverage for the script paths that previously failed: shell/Python syntax, empty/no-match fingerprints, malformed renderer input, documented search syntax, and live fingerprint/search execution on this PR.

@ericksoa
ericksoa merged commit 3351fbd into main May 7, 2026
9 checks passed
@wscurran wscurran added the feature PR adds or expands user-visible functionality label Jun 8, 2026
@cv
cv deleted the feat/skill-cross-issue-sweep branch June 28, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature PR adds or expands user-visible functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants