You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by ak...@apache.org on 2026/05/08 07:05:29 UTC
(tooling-agents) branch main updated: read_and_triage v2.2: prior-comment ingestion, skip explanations, label fixes, quality refinements
This is an automated email from the ASF dual-hosted git repository.
andrewmusselman pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-agents.git
The following commit(s) were added to refs/heads/main by this push:
new 78dfe8b read_and_triage v2.2: prior-comment ingestion, skip explanations, label fixes, quality refinements
78dfe8b is described below
commit 78dfe8bc6770567e9ac4a2a45e5119698192d5e6
Author: Andrew Musselman <ak...@apache.org>
AuthorDate: Fri May 8 00:04:50 2026 -0700
read_and_triage v2.2: prior-comment ingestion, skip explanations, label fixes, quality refinements
Prior comments on the issue now feed deep analysis as context. Skip
paths post explanation comments so users do not see a label without
any visible reasoning. Label default works correctly with the
gofannon UI submission semantics. Citation grounding is stricter,
inventory parsing more robust, domain partitions consolidated when
the model overshoots the cap. Observability prints added throughout.
Prior discussion ingestion (main behavioral change):
- New _format_issue_comments helper renders non-bot, non-self
comments chronologically into a Prior discussion block in the
analysis prompt. Filters bots (user.type=Bot, login ending in
[bot]) and the agent own sentinel-tagged comments so re-runs do
not feed the model its own prior analyses.
- 30K total cap, 2.5K per comment, oldest-dropped-first when over
budget so the freshest discussion always survives.
- _ANALYSIS_SYSTEM gains a Prior Discussion Context section telling
the model to treat in-thread decisions (library choices, blockers,
external work filed) as more current than the issue body, cite
commenters by name, and lead with in-flight external work rather
than proposing duplicate internal work.
- Tested against ATR #1154 (SWHID): previous run proposed
reimplementing the algorithm in pure Python; new run correctly
identifies that @andrewmusselman is wrapping swhid-rs as asfswhid,
names the upstream blocker (swhid-rs#44 gitoxide merge), and
proposes no internal changes.
Skip-path commenting:
- New SENTINEL_SKIP and _build_skip_comment.
- pr_in_progress skips now post a brief comment naming the in-flight
PR(s) and explaining why no full triage was run.
- Idempotent via the new sentinel; re-runs check for SENTINEL_SKIP
before posting.
- Related-cluster info for the same issue is folded into the same
skip comment under a Related issues section rather than posted as
a second comment.
- Sentinel-skipped issues unchanged; the existing v2 triage comment
from the prior run is sufficient.
In-flight PR detection and diff suppression:
- Patch Discipline section in _ANALYSIS_SYSTEM forbids no-op
placeholders. When uncertain about a specific value (enum case,
constant, signature) the model must write a TODO comment and
route the question to open_questions instead of writing
self-assignments like x.field = x.field that look like working
code on first read.
- Same section instructs the model to set diffs to empty list when
prior discussion identifies a PR already implementing the work.
- Rendering-side belt-and-suspenders: _build_comment scans summary
and approach for PR #N references not in linked_prs (the
close-keyword-detected set) and suppresses Suggested patches in
favor of an In-flight work notice naming the PR(s).
Label default sentinel:
- Empty input from the gofannon UI now defaults to gh-helper
instead of disabling labeling. The previous in-input-dict check
was always true because gofannon submits declared input fields
even when blank.
- To explicitly disable labeling pass none, off, disabled, or -
(case-insensitive).
- ensure_repo_label and add_issue_label now log on every code path
(success, no-op, exception). Top-of-loop print confirms whether
labeling is enabled and which label name will be applied.
Citation grounding refinements:
- snippet.rstrip on trailing whitespace before line splitting so
trailing newlines do not inflate computed line counts.
- Citations whose snippets contain no def / class / async def /
decorator anchor are dropped. Previous behavior was to fall back
to the longest line, which let prose explanations through as if
they were source citations.
- Per-issue print: Issue #N: Citation grounding: kept X, dropped Y.
Inventory parser robustness:
- Header regex accepts H2 through H4 (was H3 only).
- Strips markdown decorations from header text including bold,
inline code, and italic.
- Strips File: / Path: / FILE / PATH prefixes some prompts produce.
- Logs unresolved-header counts when model output does not match
any known file path.
Domain partition consolidation:
- Prompt strengthened with a STRICT REQUIREMENT to produce between
lo and hi domains total.
- Post-process: if the model overshoots the cap, keep the hi-1
largest partitions and merge the rest into a single shared bucket
with deduplicated files and concatenated concerns.
- Print: Domains: model produced X partitions, over the hi cap;
consolidated K smaller ones into shared.
Observability:
- Per-issue prints for citation grounding kept/dropped, in-flight
PR detection warnings, label apply success/failure.
- Per-run prints for inventory unresolved-header counts, domain
consolidation triggers, and a labeling-enabled banner.
Tests at 41/41 (was 36). New: T37 blank-label-defaults,
T38 PR-mention-suppresses-diffs, T39 normal-diffs-render-without-
mention, T40 pr_in_progress-posts-explanation, T41 pr_in_progress-
skip-idempotent. Updated: T26 for the new label sentinel semantics,
T21 for the combined skip-plus-related comment shape.
Per-call parameters dict (max_tokens 32768, reasoning_effort high,
temperature 1) left in for Opus deep-analysis as a fallback in case
the gofannon wrapper is fixed later. The wrapper currently ignores
per-call parameters and only honors UI-configured Invokable Model
parameters; gofannon-side issue filed separately.
---
gh-helper/README.md | 17 +-
gh-helper/agents/triage-and-pr.py | 572 ++++++++++++++++++++++++++++++++++----
2 files changed, 530 insertions(+), 59 deletions(-)
diff --git a/gh-helper/README.md b/gh-helper/README.md
index 423faa8..072558a 100644
--- a/gh-helper/README.md
+++ b/gh-helper/README.md
@@ -10,19 +10,19 @@ For each repo (cached per HEAD SHA, run once and shared across all issues):
1. **Downloads the repo as a tarball** and caches files in `data_store`. Re-runs at the same SHA skip the download.
2. **Architecture discovery** — one Sonnet call produces a structured codebase map: framework, language, purpose, auth systems, API layers, data layer, execution model, key subsystems, trust model. Files are sorted by architectural informativeness (configs, entry points, then keyword matches) so the budget always covers the most signal-dense files.
-3. **Domain partitioning** — one Sonnet call groups the codebase into application areas. The target range scales with file count (3–6 for small codebases, up to 12–30 for very large ones), so a 600-file repo gets ~7–14 domains while a 5,000-file monorepo gets ~10–20.
+3. **Domain partitioning** — one Sonnet call groups the codebase into application areas. The target range scales with file count (3–6 for small codebases, up to 12–30 for very large ones), so a 600-file repo gets ~7–14 domains while a 5,000-file monorepo gets ~10–20. If the model overshoots the cap, the smallest partitions are consolidated into a single `shared` bucket so downstream relevance scoring stays focused.
4. **File inventory** — Sonnet, batched, builds a per-file structured catalog: purpose, public API with line ranges, concerns. Cache namespace `audit-cache:inventory:{file-set-hash}` is shared with `asvs_audit`, so a repo already audited gets free hits here, and vice versa.
5. **Open-PR scan** — one `/search/issues` call enumerates open PRs and parses titles + bodies for close-keywords (`fixes #N`, `closes #N`, `resolves #N`). Builds a map of issues that already have someone working on them.
6. **Issue clustering** — one LLM call groups open issues into duplicate/related clusters. Cached by issue-corpus content.
Then for each issue:
-1. **Sentinel and PR-link checks** — skip issues already triaged by this agent, or skip with classification `pr_in_progress` if a PR is linked.
+1. **Sentinel and PR-link checks** — skip issues already triaged by this agent, or skip with classification `pr_in_progress` if a PR is linked. For `pr_in_progress`, a brief explanatory comment is posted (idempotent via its own sentinel) noting which PR(s) the agent identified as in-flight, so the user isn't left wondering why the issue got a label without an explanation. Sentinel-skipped issues already have a full triage comment from the prior run, so no new comment is posted there.
2. **Domain classification** — one Haiku call routes the issue to 1–3 application domains. Narrows the relevance-search to those domains.
3. **Relevance scoring** — Haiku, batched, scores files against the issue using *inventory entries* (not raw previews). Returns 0–10 per file. Top-K (8) selected with threshold ≥4, falling back to ≥2 if too few clear the bar.
4. **Staleness metrics** — computed locally from issue timestamps and comment activity. No LLM cost. Captures: days since created, days since last update, days since last comment, days since last *human* comment (skips bots and the agent's own comments), human comment count.
-5. **Deep analysis** — Opus, with the full structured context: architecture, domain context, inventory entries, full source of picked files, and the staleness metrics. Produces structured JSON: classification, summary, existing-code citations, new-code locations, approach, diffs, open questions, and a staleness assessment with `recommend_close`.
-6. **Citation grounding** — every existing-code citation has its line range computed from the actual source by anchor-matching. The model's claimed line range is replaced with the real one. Citations whose snippets don't appear in the cited file at all (genuine hallucinations) are dropped.
+5. **Deep analysis** — Opus, with the full structured context: architecture, domain context, inventory entries, full source of picked files, staleness metrics, and **any prior human comments on the issue** (chronologically ordered, with bot and self-sentinel comments filtered out). Produces structured JSON: classification, summary, existing-code citations, new-code locations, approach, diffs, open questions, and a staleness assessment with `recommend_close`. When the issue thread already [...]
+6. **Citation grounding** — every existing-code citation has its line range computed from the actual source by anchor-matching on the snippet's first `def`/`class`/`async def`/`@decorator` line. The model's claimed line range is replaced with the real one. Citations whose snippets contain no def/class anchor (typically prose explanations or usage examples masquerading as source citations) are dropped, as are citations whose snippets don't appear in the cited file at all (genuine hallucin [...]
7. **Comment** — renders all of the above as markdown with sections for Where this lives, Where new code would go, Approach, Suggested patches, Open questions, Staleness assessment (only when applicable). Posts to GitHub or returns the draft if `dry_run` is true.
8. **Label** — applies the configured label to the issue (creates the label in the repo on first run if missing).
@@ -145,7 +145,7 @@ A few practical notes on the more nuanced flags:
- **`skip_already_triaged`**: leave true for normal operation. Only set false when intentionally re-triaging (after a prompt change). Even then, run with `dry_run` first — re-triaged issues get a *new* comment alongside the old one, not a replacement.
- **`skip_when_pr_open`**: keep true unless you want to re-evaluate issues that already have an open PR. With true, those issues get classification `pr_in_progress` and a small "see also" comment only if related-issue clustering finds duplicates.
- **`detect_related_issues`**: keep true. The cluster pass costs about one Haiku call per issue corpus and pays for itself by finding duplicates the per-issue analysis can't see.
-- **`label`**: set to `""` to disable labeling entirely. Otherwise the agent ensures the label exists in the repo and applies it to every examined issue.
+- **`label`**: defaults to `gh-helper`. Pass `none` (case-insensitive; `off`, `disabled`, and `-` also work) to disable labeling entirely. An empty string is treated as "use default" rather than "disable" — gofannon's UI submits empty strings for blank fields, so checking presence-of-key isn't sufficient. Pass any other string to use a custom label name; it's created in the repo on first run if missing.
## Inputs reference
@@ -160,7 +160,7 @@ A few practical notes on the more nuanced flags:
| `assignee` | string | no | `""` | filter to issues assigned to this GitHub username; `*` = any assignee, `none` = unassigned only, empty = no filter |
| `skip_when_pr_open` | boolean | no | `true` | skip issues with open PRs that close-keyword-link them; classification `pr_in_progress` |
| `detect_related_issues` | boolean | no | `true` | run the cross-issue clustering pass; false saves one LLM call per corpus |
-| `label` | string | no | `gh-helper` | label applied to every examined issue; created in the repo if missing; empty disables labeling |
+| `label` | string | no | `gh-helper` | label applied to every examined issue; created in the repo if missing. Pass `none` (or `off`/`disabled`/`-`) to disable. Empty string defaults to `gh-helper` since the gofannon UI submits empty strings for blank fields |
Ten inputs total: 2 required, 8 optional. Model selection, cost limits (max issues, max files per issue), and reasoning configuration are all hardcoded internally — model identities match the Invokable Models registration, and per-model parameters (temperature, max_tokens, reasoning_effort) are configured in the gofannon UI.
@@ -203,7 +203,7 @@ Triage comments include these sections, in order:
- **Type / Classification / Confidence** — bug_fix / new_feature / refactor / documentation / question / discussion / unrelated / unclear; classification of actionable / no_action / unrelated; confidence high/medium/low. **Stale flag** (⚠️ Stale — consider closing) appears in this header line when the agent recommends closing.
- **Application domain(s)** — which area(s) of the codebase
-- **Summary** — what the issue asks, what was found
+- **Summary** — what the issue asks, what was found. When the issue thread already contains team decisions, the summary attributes them to the commenters who made them (`@username confirmed...`, `@username noted...`).
- **Where this lives in the code today** — for each cited piece of existing code: file path, symbol name, line range (computed from actual source by snippet match), role (currently does this / needs modification / extension point), explanation, and a verbatim code snippet
- **Where new code would go** (only for new-feature issues) — file + anchor + rationale
- **Proposed approach** — paragraph
@@ -225,6 +225,8 @@ The cost shape: discovery is roughly 1–3 minutes per SHA on first encounter, t
The staleness check piggybacks on the deep-analysis call — no extra LLM round-trip. Age metrics are computed locally from data already in scope (issue timestamps, comment timestamps from the sentinel-check fetch). The model integrates these signals with its code analysis to assess whether an issue's premise still holds.
+Prior-discussion ingestion also piggybacks on the same fetch — comments retrieved for the sentinel check are reused as context for the deep-analysis prompt, with bots and the agent's own past triages filtered out. This means the analysis incorporates whatever the team has decided in-thread (library choices, blockers, external in-flight work) and cites commenters by name, rather than ignoring the discussion and proposing alternatives the team has already considered and rejected.
+
## Caches and namespaces
The agent uses these `data_store` namespaces. They're created on first write — you don't need to declare them anywhere. Worth knowing for debugging or manual cache clearing through the gofannon data-store viewer:
@@ -249,6 +251,7 @@ The `files:{repo}` and `audit-cache:inventory:*` namespaces match what `asvs_aud
- **Staleness signals.** The agent skips its own comments and other bots when computing "last human activity" — so the agent's own re-triage doesn't reset the staleness clock. Activity by named human accounts is what counts.
- **Recommended close ≠ auto-close.** The agent only *recommends* closure in the comment body and surfaces `recommend_close: true` in the result row. The agent never closes issues programmatically. A maintainer reading the comment makes the call.
- **Per-user runs.** Pass `assignee` with a GitHub username to triage only that user's assigned issues. Combined with `dry_run: true` for a personal sanity check before the bot posts anywhere.
+- **Diagnostic prints.** The agent emits a few log lines worth knowing for debugging: `Inventory: N header(s) didn't resolve to a known file (e.g. ...)` flags model output that didn't match any catalog entry; `Domains: model produced X partitions, over the {hi} cap — consolidated K smaller ones into 'shared'` fires when the partition consolidation kicks in; `Issue #N: Citation grounding: kept X, dropped Y` reports how many citations survived anchor matching per issue; and per-issue `Labe [...]
## Roadmap
diff --git a/gh-helper/agents/triage-and-pr.py b/gh-helper/agents/triage-and-pr.py
index 630c39c..f493ab2 100644
--- a/gh-helper/agents/triage-and-pr.py
+++ b/gh-helper/agents/triage-and-pr.py
@@ -63,6 +63,12 @@ async def run(input_dict, tools):
# Separate sentinel so the two comment types are independently
# idempotent.
SENTINEL_RELATED = "<!-- gofannon-issue-triage-bot v2 related -->"
+ # Posted on issues skipped due to an open PR linking to them
+ # (classification `pr_in_progress`). Without this, the user sees a
+ # `gh-helper` label but no comment explaining why no full triage was
+ # done, which is confusing. Independently sentinel'd so re-runs don't
+ # double-post.
+ SENTINEL_SKIP = "<!-- gofannon-issue-triage-bot v2 skip -->"
# ---------- module-level helpers ----------
@@ -569,7 +575,7 @@ Rules:
- Each file should appear in EXACTLY ONE domain.
- Configs, migrations, shared utilities go in an "infrastructure" or "shared" domain.
- Use snake_case names. Be specific to this codebase (not generic categories).
-- Stay between {lo} and {hi} domains total.
+- STRICT REQUIREMENT: produce between {lo} and {hi} domains TOTAL — no exceptions. If you find yourself wanting more than {hi} domains, COMBINE related ones (e.g., merge "voting", "ballot_management", and "vote_results" into one "voting" domain). Smaller is better than fragmented.
FILE LIST:
"""
@@ -648,6 +654,54 @@ FILE LIST:
"files": files,
"concerns": [str(c).strip() for c in (d.get("concerns") or [])],
})
+
+ # Post-process: if the model overshot the cap, consolidate
+ # the smallest domains into a shared "shared" bucket. The
+ # smallest domains are usually scattershot grab-bags or
+ # over-eager splits that can be safely merged. We keep the
+ # `hi - 1` largest domains as-is and merge the rest into
+ # a single shared bucket. Better to have ONE bucket that's
+ # slightly less specific than to have N tiny domains the
+ # downstream relevance pass can't usefully scope on.
+ if len(cleaned_domains) > hi:
+ cleaned_domains.sort(key=lambda d: len(d["files"]),
+ reverse=True)
+ keepers = cleaned_domains[:hi - 1]
+ overflow = cleaned_domains[hi - 1:]
+ merged_files = []
+ merged_concerns = []
+ merged_names = []
+ for d in overflow:
+ merged_files.extend(d["files"])
+ merged_names.append(d["name"])
+ for c in d.get("concerns", []):
+ if c and c not in merged_concerns:
+ merged_concerns.append(c)
+ # Deduplicate files (a file should not appear twice).
+ seen_files = set()
+ deduped = []
+ for f in merged_files:
+ if f not in seen_files:
+ seen_files.add(f)
+ deduped.append(f)
+ shared_bucket = {
+ "name": "shared",
+ "description": (
+ "Consolidated bucket of smaller subsystems "
+ "merged together: " + ", ".join(merged_names[:8])
+ + ("…" if len(merged_names) > 8 else "")
+ ),
+ "files": deduped,
+ "concerns": merged_concerns[:20],
+ }
+ cleaned_domains = keepers + [shared_bucket]
+ print(
+ f" Domains: model produced {len(keepers) + len(overflow)} "
+ f"partitions, over the {hi} cap — consolidated "
+ f"{len(overflow)} smaller ones into 'shared'",
+ flush=True,
+ )
+
result = {"domains": cleaned_domains}
cache_ns.set("domains", result)
print(f" Domains: {len(cleaned_domains)} partitions", flush=True)
@@ -781,18 +835,54 @@ FILES:
*[_one_batch(i, b) for i, b in enumerate(batches)]
)
- # Parse: each file's block starts with "### path/to/file.py"
+ # Parse: each file's block starts with a header line that
+ # names the path. The model is supposed to produce
+ # "### path/to/file.py" but in practice often emits
+ # "## path", "#### path", "### **path**", "### `path`",
+ # or "### File: path" — all mean the same thing. We accept
+ # any header level 2-4, strip markdown decorations, and
+ # validate against all_files. Files that come back missed
+ # by the parser are the single biggest cause of inventory
+ # under-coverage on real runs.
+ _HEADER_RE = re.compile(r'^#{2,4}\s+(.+?)\s*$')
+ _PREFIX_RE = re.compile(
+ r'^(?:File|Path|FILE|PATH)\s*:\s*', re.IGNORECASE,
+ )
inventory = {}
current_path = None
current_block = []
+ misses = []
for chunk in results:
for line in chunk.split('\n'):
- m = re.match(r'^###\s+(.+?)\s*$', line)
+ m = _HEADER_RE.match(line)
if m:
if current_path and current_path in all_files:
inventory[current_path] = '\n'.join(current_block).strip()
candidate = m.group(1).strip()
- current_path = candidate if candidate in all_files else None
+ # Strip "File: " / "Path: " prefix if present.
+ candidate = _PREFIX_RE.sub('', candidate)
+ # Strip markdown decorations: bold, italic,
+ # code, surrounding punctuation.
+ candidate = candidate.strip().strip('*_`"\'')
+ # Some models wrap in **`path`** — strip again.
+ candidate = candidate.strip().strip('*_`"\'')
+ if candidate in all_files:
+ current_path = candidate
+ else:
+ current_path = None
+ # Track misses so we can debug parser
+ # coverage later.
+ if candidate and (
+ candidate.endswith(
+ ('.py', '.ts', '.tsx', '.js', '.jsx',
+ '.go', '.java', '.rs', '.rb', '.kt',
+ '.swift', '.c', '.cpp', '.h', '.hpp',
+ '.toml', '.yaml', '.yml', '.json',
+ '.md', '.html', '.css')
+ )
+ or '/' in candidate
+ ):
+ misses.append(candidate)
current_block = []
else:
if current_path:
@@ -803,6 +893,15 @@ FILES:
cache_ns.set("triage_inventory", inventory)
print(f" Inventory: cataloged {len(inventory)} files",
flush=True)
+ if misses:
+ # Show the first few that didn't resolve, so we can
+ # see what shape the parser is missing.
+ preview = ", ".join(misses[:5])
+ print(
+ f" Inventory: {len(misses)} header(s) didn't "
+ f"resolve to a known file (e.g. {preview})",
+ flush=True,
+ )
return inventory
@@ -1314,6 +1413,84 @@ FILE INVENTORIES:
return "\n".join(lines)
+ def _build_skip_comment(skip_reason, skip_summary,
+ linked_pr_list, related_info,
+ *, head_sha, branch, sentinel):
+ """Brief explanation comment for issues we're skipping with a
+ label applied. Without this the user sees a `gh-helper` label
+ and no comment, which looks like the agent malfunctioned.
+
+ Currently only emitted for `pr_in_progress` skips — sentinel
+ skips already have a full v2 triage comment from the prior
+ run, so a new explanation would just be noise.
+
+ If related-issue clustering also surfaced something for this
+ issue, fold it into the same comment rather than posting two.
+ """
+ lines = [
+ sentinel,
+ "",
+ f"_Automated triage — analyzed at `{branch}@{head_sha[:8]}`_",
+ "",
+ ]
+ if skip_reason == "pr_in_progress" and linked_pr_list:
+ pr_refs = ", ".join(
+ f"#{p['number']}" for p in linked_pr_list
+ )
+ plural = "s" if len(linked_pr_list) > 1 else ""
+ lines.append(
+ f"**Classification:** `pr_in_progress`"
+ )
+ lines.append("")
+ lines.append(
+ f"This issue appears to be addressed by open "
+ f"PR{plural}: {pr_refs}. The agent skipped detailed "
+ f"code analysis to avoid duplicating effort."
+ )
+ lines.append("")
+ lines.append(
+ f"Re-run triage after the PR{plural} merge"
+ f"{'s' if not plural else ''} or close"
+ f"{'s' if not plural else ''} if you want a fresh "
+ f"analysis from the agent."
+ )
+ else:
+ # Generic fallback — should not normally happen since
+ # we only call this for pr_in_progress, but defend
+ # against future skip reasons being added.
+ lines.append(f"**Classification:** `{skip_reason}`")
+ lines.append("")
+ lines.append(skip_summary or
+ "The agent skipped detailed analysis "
+ "of this issue.")
+ # Fold related-cluster info into the same comment if present.
+ if related_info:
+ others = related_info.get("others") or []
+ if others:
+ refs = ", ".join(f"#{n}" for n in others)
+ kind = related_info.get("kind", "related")
+ label_text = (
+ "may be a **duplicate** of (or share root cause with)"
+ if kind == "duplicate"
+ else "appears **related** to"
+ )
+ lines.append("")
+ lines.append(f"### Related issues")
+ lines.append("")
+ lines.append(f"This issue {label_text}: {refs}.")
+ rationale = related_info.get("rationale", "")
+ if rationale:
+ lines.append("")
+ lines.append(f"**Rationale:** {rationale}")
+ lines.append("")
+ lines.append("---")
+ lines.append(
+ "*Draft from a triage agent. A human reviewer should "
+ "validate.*"
+ )
+ return "\n".join(lines)
+
+
# ---------- staleness assessment ----------
def _compute_staleness_metrics(issue, comments):
@@ -1424,12 +1601,27 @@ For each diff you propose:
- Anchor it to a real file. If the file doesn't exist in the repo, mark it as a new file.
- Make diffs surgical — show ONLY the changed regions, not the whole file.
- Keep diffs minimal and clearly motivated. They are starting points for human review.
+- **NEVER write no-op placeholders.** If you are uncertain about a specific value (an enum case you can't verify, a constant, a method signature, the exact name of a field), write `# TODO: confirm <thing>` and place the question in `open_questions`. A line like `x.field = x.field` (assigning a value to itself) or `# Mark as X if Y exists` followed by a self-assignment is worse than no diff at all — it looks like working code on first read and silently does nothing. When you don't know an [...]
+- **Defer to in-flight work.** If the prior discussion identifies a PR that already implements (or is implementing) what this issue asks for — e.g., "@user noted PR #1186 is ready for review" — DO NOT propose your own diff. Instead, set `diffs` to `[]`, mention the PR in your `summary` and `approach`, and let the maintainer review the actual PR. Producing an illustrative diff in this case wastes reviewer time comparing your guess to the real implementation.
## What NOT to do
- Don't invent function names. If you can't see something in the inventory or source, say so.
- Don't propose patches against files you haven't read the source for.
- Don't claim the issue is `actionable` just because you can write SOME diff. Lower-confidence diffs go in `open_questions` instead, with `classification` of `no_action`.
+## Prior Discussion Context
+You will be given any prior comments on the issue, in chronological order with author and timestamp. **Read the prior discussion CAREFULLY before forming your analysis.** The discussion is often the most current source of truth — more so than the issue body, which reflects the original framing and may be stale.
+
+If the team has, in the discussion:
+- **Already chosen a library, package, or implementation approach** (e.g., "we'll wrap the Rust reference impl rather than reimplement", "we're using package X"): your analysis must build on that decision, not propose alternatives that were already considered and rejected.
+- **Identified blockers** (licensing conflicts, security concerns, upstream dependencies): cite the blocker and route around it.
+- **Filed related external work** (PRs, sister repos, upstream contributions): mention it explicitly and treat it as the locus of work, not propose duplicating it inside this repo.
+- **Decided to wait** (for upstream merges, legal reviews, beta milestones): respect that — don't push for immediate action.
+
+When the discussion contradicts the issue body, defer to the discussion. When you cite something the team decided, name the commenter (e.g., "@username noted that swh.model is GPL-3 and unusable"). When in-flight external work exists, lead with it in your summary rather than proposing new internal work.
+
+If there is no prior discussion (fresh issue), proceed from the body alone.
+
## Staleness Assessment
For every issue, also assess whether it appears stale and whether closing it should be considered.
@@ -1513,9 +1705,99 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
return "\n".join(parts)
+ def _format_issue_comments(comments,
+ *,
+ max_total_chars=30000,
+ max_comment_chars=2500):
+ """Render non-bot comments as a chronological discussion
+ thread for inclusion in the deep-analysis prompt.
+
+ Filters out:
+ - Bots (user.type == "Bot", login ending in [bot])
+ - The agent's own sentinel-tagged comments (otherwise
+ re-running on the same issue would feed the model
+ its own prior analyses)
+
+ Comments are sorted oldest-first so the model reads the
+ thread the way a human reviewer would. Long comments
+ are truncated at max_comment_chars; total output is
+ capped at max_total_chars by dropping the OLDEST
+ comments (so the freshest context survives), with a
+ marker noting how many were omitted.
+
+ Returns a formatted markdown string, or empty string if
+ no relevant comments exist.
+ """
+ from datetime import datetime, timezone
+
+ if not comments:
+ return ""
+
+ # Filter
+ relevant = []
+ for c in comments:
+ body = c.get("body") or ""
+ if "<!-- gofannon-issue-triage-bot" in body:
+ continue
+ user = c.get("user") or {}
+ user_type = (user.get("type") or "").lower()
+ login = (user.get("login") or "").lower()
+ if user_type == "bot" or login.endswith("[bot]"):
+ continue
+ relevant.append(c)
+
+ if not relevant:
+ return ""
+
+ # Sort chronologically (oldest first)
+ def _ts(c):
+ s = c.get("created_at") or ""
+ try:
+ return datetime.fromisoformat(s.replace("Z", "+00:00"))
+ except Exception:
+ return datetime.min.replace(tzinfo=timezone.utc)
+ relevant.sort(key=_ts)
+
+ # Format. We build from newest backward so that when we
+ # hit the size cap, the OLDEST comments are dropped
+ # (the freshest discussion is what tells the agent the
+ # current state of the work).
+ blocks = []
+ total = 0
+ dropped = 0
+ for c in reversed(relevant):
+ body = (c.get("body") or "").strip()
+ if not body:
+ continue
+ if len(body) > max_comment_chars:
+ body = (
+ body[:max_comment_chars]
+ + "\n[... comment truncated ...]"
+ )
+ author = (c.get("user") or {}).get("login") or "(unknown)"
+ ts = c.get("created_at") or "(no timestamp)"
+ block = f"### Comment by @{author} ({ts})\n{body}\n"
+ if total + len(block) > max_total_chars and blocks:
+ dropped = len(relevant) - len(blocks)
+ break
+ blocks.append(block)
+ total += len(block)
+
+ # blocks is newest-first; reverse to chronological order
+ blocks.reverse()
+ if dropped:
+ blocks.insert(
+ 0,
+ f"_[{dropped} earlier comment(s) omitted for length; "
+ f"showing most recent {len(blocks)}]_\n",
+ )
+ return "\n".join(blocks)
+
+
def _analysis_prompt(repo, head_sha, number, title, body, html_url,
file_blobs, architecture, issue_domains,
- domains_dict, inventory, staleness_metrics):
+ domains_dict, inventory, staleness_metrics,
+ issue_comments):
files_section_parts = []
for fb in file_blobs:
files_section_parts.append(
@@ -1557,6 +1839,18 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
f"- Human comments on issue: {sm.get('human_comment_count', 0)}",
])
+ # Render the prior discussion. When there are no prior
+ # comments (fresh issue), the section is omitted entirely
+ # so the model isn't confused by an empty header.
+ discussion = _format_issue_comments(issue_comments)
+ if discussion:
+ discussion_block = (
+ f"## Prior discussion (chronological, oldest first)\n\n"
+ f"{discussion}\n\n"
+ )
+ else:
+ discussion_block = ""
+
return (
f"REPOSITORY: {repo} @ {head_sha[:8]}\n\n"
f"# Architecture\n{arch_text}\n\n"
@@ -1568,12 +1862,20 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
f"URL: {html_url}\n\n"
f"## Issue age and activity\n{age_block}\n\n"
f"## Issue body\n{(body or '(no body)')[:8000]}\n\n"
+ f"{discussion_block}"
f"# Source files (full content)\n\n{files_section}\n\n"
f"# Your Task\n"
f"Follow the system instructions to produce a structured "
- f"JSON triage. Cite real code with line ranges. Assess "
- f"staleness using both the age/activity metrics and your "
- f"code analysis. Be honest about what you do not understand."
+ f"JSON triage. CRITICAL: read the prior discussion "
+ f"section above carefully BEFORE forming your analysis. "
+ f"If the team has already chosen a library, identified "
+ f"a blocker, filed related work, or rejected a particular "
+ f"approach, your analysis MUST reflect those decisions. "
+ f"Cite specific commenters in your summary when relevant "
+ f"(e.g., '@username noted that...'). Then cite real code "
+ f"with line ranges, assess staleness using the age/"
+ f"activity metrics and your code analysis, and be honest "
+ f"about what you do not understand."
)
@@ -1672,15 +1974,17 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
}
- def _ground_citations(parsed, all_files):
+ def _ground_citations(parsed, all_files, *, issue_number=None):
"""Replace LLM-claimed line ranges with ranges computed by
matching the actual snippet text against the source file.
Drops citations whose snippets don't appear in the cited
file (real text hallucinations, distinct from line-number
- errors). Keeps citations where we can match the snippet
- even if the line range was wrong — fixes the line range,
- preserves the substance.
+ errors). Drops citations whose snippet has no def/class/
+ decorator anchor — those are usually prose explanations or
+ usage examples masquerading as source citations, and the
+ longest-line fallback used to silently let them through
+ with the model's claimed line range intact.
The model's snippet text is verbatim accurate in practice
(verified against ATR source); only its line annotations
@@ -1701,11 +2005,28 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
dropped += 1
continue
source = all_files[path]
- snippet_lines = snippet.split("\n")
- # Pick an anchor line we can search for. Prefer
- # def/class/decorator lines (typically unique within
- # a file). Fall back to the longest non-trivial line.
+ # Strip trailing newlines/whitespace before splitting so
+ # the line count reflects actual content, not artifacts
+ # of model output formatting or JSON serialization.
+ # Without this, a snippet ending in many \n chars
+ # produces a giant len(snippet_lines) that makes
+ # end_line wildly overshoot.
+ snippet_clean = snippet.rstrip("\n\r\t ")
+ snippet_lines = snippet_clean.split("\n")
+ # Also drop any leading blank lines.
+ while snippet_lines and not snippet_lines[0].strip():
+ snippet_lines.pop(0)
+ if not snippet_lines:
+ dropped += 1
+ continue
+
+ # Anchor on a def/class/decorator line. If the snippet
+ # has no such line, it's almost certainly prose or a
+ # usage example, not a real source citation — drop it
+ # rather than guess. This catches cases like
+ # "async with db.ensure_session(caller_data) as data:"
+ # being passed off as a function-definition snippet.
anchor = None
for line in snippet_lines:
stripped = line.strip()
@@ -1715,13 +2036,7 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
anchor = line
break
if not anchor:
- non_empty = [l for l in snippet_lines if l.strip()]
- if non_empty:
- anchor = max(non_empty, key=len)
-
- if not anchor or not anchor.strip():
- cite["lines"] = "(line range not verified)"
- grounded.append(cite)
+ dropped += 1
continue
# Search for the anchor in the source. Try exact match
@@ -1738,7 +2053,6 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
if idx == -1:
# Genuine hallucination: snippet content not in file.
- # Don't render this citation.
dropped += 1
continue
@@ -1747,12 +2061,13 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
cite["lines"] = f"{start_line}-{end_line}"
grounded.append(cite)
- if dropped:
- print(
- f" Citation grounding: dropped {dropped} unverifiable "
- f"citation(s) (snippet not found in source)",
- flush=True,
- )
+ tag = (f"Issue #{issue_number}: " if issue_number is not None
+ else "")
+ print(
+ f" {tag}Citation grounding: kept {len(grounded)}, "
+ f"dropped {dropped}",
+ flush=True,
+ )
parsed["existing_code"] = grounded
return parsed
@@ -1760,18 +2075,56 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
def _build_comment(parsed, *, head_sha, branch, files_examined,
sentinel, related_info=None, issue_domains=None,
- staleness_metrics=None):
+ staleness_metrics=None, issue_number=None,
+ linked_prs=None):
"""v2 comment format: structured sections for existing-code
citations, new-code locations, approach, patches, open questions.
Includes a staleness assessment section when the model
recommends closing or flags the issue as stale.
+
+ If the analysis text mentions a PR (e.g., '@user noted PR #1186
+ is ready for review') that wasn't caught by close-keyword
+ scanning, suggested patches are suppressed and an 'In-flight
+ work' notice is rendered instead. The model is instructed
+ via the system prompt to defer in this case, but this
+ rendering-side check is a belt-and-suspenders safety net
+ for when the model produces diffs anyway.
"""
+ import re as _re
+
triage_type = parsed.get("triage_type", "unclear")
cls = parsed["classification"]
conf = parsed["confidence"]
summary = parsed["summary"]
staleness = parsed.get("staleness") or {}
+ # Detect PR mentions in the analysis text not covered by
+ # close-keyword detection. Matches "PR #N" / "PR N" /
+ # "pull request #N" (case-insensitive).
+ text_to_scan = (
+ (parsed.get("summary") or "")
+ + " "
+ + (parsed.get("approach") or "")
+ )
+ pr_mentions = set()
+ for m in _re.finditer(
+ r"(?i)\b(?:pr|pull request)s?\s*#?(\d+)\b",
+ text_to_scan,
+ ):
+ n = int(m.group(1))
+ # Don't treat the issue's own number as a PR mention.
+ if issue_number is not None and n == issue_number:
+ continue
+ pr_mentions.add(n)
+ already_linked = set(linked_prs or [])
+ unlinked_pr_mentions = sorted(pr_mentions - already_linked)
+ if unlinked_pr_mentions and issue_number is not None:
+ print(
+ f" Issue #{issue_number}: analysis mentions "
+ f"PR(s) {unlinked_pr_mentions} not detected by "
+ f"close-keyword scan — suppressing diffs"
+ )
+
domain_names = (
", ".join(f"`{e['name']}`" for e in (issue_domains or []))
if issue_domains else "_(none identified)_"
@@ -1845,7 +2198,26 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
lines.append("")
diffs = parsed.get("diffs") or []
- if cls == "actionable" and diffs:
+ if unlinked_pr_mentions and (cls == "actionable") and diffs:
+ # Don't propose diffs when the analysis identifies
+ # in-flight PR work — the maintainer should review the
+ # actual PR(s) rather than compare them to an
+ # illustrative diff. Render a compact notice so it's
+ # clear *why* there are no patches.
+ lines.append("### In-flight work")
+ lines.append("")
+ pr_list = ", ".join(
+ f"#{n}" for n in unlinked_pr_mentions
+ )
+ plural = "s" if len(unlinked_pr_mentions) > 1 else ""
+ lines.append(
+ f"The analysis identifies PR{plural} {pr_list} as "
+ f"in-flight work for this issue. Suggested patches "
+ f"are suppressed so reviewers focus on the actual "
+ f"PR{plural} rather than an illustrative diff."
+ )
+ lines.append("")
+ elif cls == "actionable" and diffs:
lines.append("### Suggested patches")
lines.append("")
for d in diffs:
@@ -2023,10 +2395,22 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
# an empty string to disable labeling entirely. Default
# "gh-helper" matches the existing convention; the agent will
# auto-create the label in the repo if it doesn't exist.
- if "label" in input_dict:
- label = (input_dict.get("label") or "").strip()
- else:
+ # Label applied to every issue the agent examines. Idempotent on
+ # GitHub's side (re-adding an existing label is a no-op).
+ #
+ # Defaulting semantics: gofannon's UI submits every declared
+ # input field even when blank, so checking for presence-of-key
+ # is insufficient — we have to default whenever the value is
+ # empty/whitespace. To explicitly DISABLE labeling, pass the
+ # sentinel 'none' (case-insensitive). 'off', 'disabled', and
+ # '-' are accepted aliases.
+ label_input = (input_dict.get("label") or "").strip()
+ if label_input.lower() in ("none", "off", "disabled", "-"):
+ label = ""
+ elif not label_input:
label = "gh-helper"
+ else:
+ label = label_input
# skip_when_pr_open: skip triage for any issue that has an open PR
# using close-keywords (fixes/closes/resolves #N). Catches the
# "someone is already working on this" case to avoid duplicate
@@ -2081,6 +2465,10 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
Swallows errors — 422 means it already exists, anything else
we log and let per-issue add calls surface their own errors.
Bypasses gh_post because gh_post raises on 4xx.
+
+ Logs success/no-op so the user can confirm the call ran.
+ Without this, a silent failure here cascades to every
+ per-issue add_issue_label call returning 422.
"""
if not label_name:
return
@@ -2096,31 +2484,60 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
),
},
)
- if r.status_code not in (201, 422):
+ if r.status_code == 201:
print(
- f"WARN: ensure_repo_label({label_name!r}) "
- f"-> {r.status_code}: {r.text[:200]}",
+ f" ensure_repo_label: created {label_name!r} "
+ f"on {owner}/{name}",
+ flush=True,
+ )
+ elif r.status_code == 422:
+ print(
+ f" ensure_repo_label: {label_name!r} already "
+ f"exists on {owner}/{name}",
+ flush=True,
+ )
+ else:
+ print(
+ f"WARN: ensure_repo_label({label_name!r}) -> "
+ f"{r.status_code}: {r.text[:300]}",
flush=True,
)
except Exception as exc:
- print(f"WARN: ensure_repo_label failed: {exc}", flush=True)
+ print(
+ f"WARN: ensure_repo_label({label_name!r}) raised: "
+ f"{type(exc).__name__}: {exc}",
+ flush=True,
+ )
async def add_issue_label(issue_number, label_name):
"""Add a label to an issue. Idempotent on GitHub's side
- (re-adding an existing label is a no-op). Wrapped in try/
- except so a label failure doesn't break the run.
+ (re-adding an existing label is a no-op).
+
+ Logs the response status on every call — without this,
+ a silent failure (e.g., the label doesn't exist at the
+ repo level) is invisible. The previous version only
+ logged on Exception, but gh_post raises on 4xx so
+ 422-because-label-doesn't-exist surfaces here, but
+ without context we couldn't tell the user WHY.
"""
if not label_name:
return
try:
- await gh_post(
+ r = await gh_post(
f"/repos/{owner}/{name}/issues/{issue_number}/labels",
json_body={"labels": [label_name]},
)
+ # gh_post raises on >=400, so reaching here means 2xx.
+ print(
+ f" Labeled #{issue_number} with {label_name!r} "
+ f"(status {r.status_code})",
+ flush=True,
+ )
except Exception as exc:
print(
- f"WARN: failed to label #{issue_number} "
- f"with {label_name!r}: {exc}",
+ f"WARN: add_issue_label(#{issue_number}, "
+ f"{label_name!r}) failed: {type(exc).__name__}: "
+ f"{exc}",
flush=True,
)
@@ -2321,7 +2738,21 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
# Idempotent and dry_run-aware. Skipping this is safe — per-issue
# add calls will report their own errors if it failed.
if label and not dry_run:
+ print(
+ f"Labeling enabled: will tag triaged issues with "
+ f"{label!r} on {owner}/{name}",
+ flush=True,
+ )
await ensure_repo_label(label)
+ else:
+ reason = (
+ "label is empty" if not label
+ else "dry_run is enabled"
+ )
+ print(
+ f"Labeling disabled ({reason}); issues will not be tagged",
+ flush=True,
+ )
results = []
errors = []
@@ -2336,7 +2767,7 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
comments_url = issue.get("comments_url") or ""
try:
- # 7a. fetch existing comments once for both sentinel checks
+ # 7a. fetch existing comments once for sentinel checks
existing_comments = []
if comments_url:
cr = await gh_get(comments_url)
@@ -2349,6 +2780,10 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
SENTINEL_RELATED in (c.get("body") or "")
for c in existing_comments
)
+ already_has_skip = any(
+ SENTINEL_SKIP in (c.get("body") or "")
+ for c in existing_comments
+ )
# 7b. cross-issue context for THIS issue
related_info = related_map.get(number)
@@ -2370,15 +2805,43 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
)
if skip_reason is not None:
- # SKIP path. We don't run relevance/analysis. We may
- # still post a standalone "see also" comment if this
- # issue is part of a duplicate cluster and we haven't
- # already posted one.
+ # SKIP path. We don't run relevance/analysis.
+ #
+ # For sentinel-skipped issues (already_triaged), there's
+ # already a v2 triage comment on the issue from a prior
+ # run, so a new explanation comment would just be noise.
+ # We may still post a standalone "see also" comment if
+ # this issue is part of a cluster and we haven't.
+ #
+ # For pr_in_progress, the user has not seen any comment
+ # from the agent, so we post a short explanation noting
+ # which PR(s) are in flight. Related-cluster info, if
+ # present, is folded into the same comment.
skipped += 1
comment_body = ""
comment_url = ""
did_post = False
- if related_others and not already_has_related:
+ if skip_reason == "pr_in_progress" and not already_has_skip:
+ # Combined skip-explanation + related comment
+ comment_body = _build_skip_comment(
+ skip_reason, skip_summary,
+ linked_pr_list,
+ related_info if (
+ related_others and not already_has_related
+ ) else None,
+ head_sha=head_sha, branch=branch,
+ sentinel=SENTINEL_SKIP,
+ )
+ if not dry_run:
+ pr_resp = await gh_post(
+ f"/repos/{owner}/{name}/issues/{number}/comments",
+ json_body={"body": comment_body},
+ )
+ comment_url = pr_resp.json().get("html_url", "")
+ did_post = True
+ elif related_others and not already_has_related:
+ # Sentinel-skip path with cluster info — post a
+ # standalone related comment as before.
comment_body = _build_related_comment(
related_info,
head_sha=head_sha, branch=branch,
@@ -2461,7 +2924,7 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
content = content[:30_000] + "\n\n[... file truncated ...]"
file_blobs.append({"path": p, "content": content})
- # 7h. deep analysis (with architecture + domains + inventory + age)
+ # 7h. deep analysis (with architecture + domains + inventory + age + prior discussion)
staleness_metrics = _compute_staleness_metrics(
issue, existing_comments
)
@@ -2469,6 +2932,7 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
repo, head_sha, number, title, body, html_url, file_blobs,
architecture, issue_domains, domains_dict, inventory,
staleness_metrics,
+ existing_comments,
)
# Opus parameters are passed at the call site rather
# than via UI config — this is the asvs_audit
@@ -2498,7 +2962,9 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
# with ranges computed by matching snippets against the
# actual source. Drops citations with snippets that
# don't appear in the cited file.
- parsed = _ground_citations(parsed, all_files)
+ parsed = _ground_citations(
+ parsed, all_files, issue_number=number,
+ )
# 7i. comment body — v2 grounded format
comment_body = _build_comment(
@@ -2509,6 +2975,8 @@ If `triage_type` is `question`, `discussion`, `unrelated`, or `unclear`: set `cl
related_info=related_info,
issue_domains=issue_domains,
staleness_metrics=staleness_metrics,
+ issue_number=number,
+ linked_prs=linked_pr_numbers,
)
# 7i. post (or skip in dry run)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org