Skip to content

fix(retro): prohibit filing "evidence for" issues - #59

Merged
rh-hemartin merged 1 commit into
mainfrom
fix/3617-no-evidence-issues
Jul 8, 2026
Merged

fix(retro): prohibit filing "evidence for" issues#59
rh-hemartin merged 1 commit into
mainfrom
fix/3617-no-evidence-issues

Conversation

@rh-hemartin

Copy link
Copy Markdown
Member

Summary

  • Add explicit prohibition against filing "Evidence for #XXXX" issues to skills/retro-analysis/SKILL.md
  • Evidence for existing issues goes in the summary field (posted as PR comment), not as a new proposal

Mirror of fullsend-ai/fullsend#3638. This is the production copy — the agents repo is what running retro agents actually load.

336 of 485 retro-filed issues since July 5 (69%) are evidence filings. The LLM found it could pass schema validation by titling proposals "Evidence for #XXXX" and using standard fields, bypassing the "skip duplicates" instruction.

Fixes fullsend-ai/fullsend#3617

Test plan

  • Next 20 retro runs produce zero "Evidence for" titled issues
  • Retro summaries posted on originating PRs still reference existing issues with specific evidence

🤖 Generated with Claude Code

@rh-hemartin
rh-hemartin requested a review from a team as a code owner July 8, 2026 14:21
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Prohibit filing “Evidence for #XXXX” issues in retro-analysis skill

📝 Documentation ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Explicitly forbid retro agents from filing “Evidence for #XXXX” proposals as new issues.
• Require corroborating evidence for existing issues to be recorded in the retro summary.
• Improve duplicate-skip notes by including issue number and concrete evidence found.
Diagram

graph TD
A[Retro agent] --> B["skills/retro-analysis/SKILL.md"] --> C{"Duplicate / evidence?"}
C -->|"Evidence for existing"| D["Write in summary"] --> E["PR comment"]
C -->|"New issue"| F["Create proposal"] --> G{{"Issue tracker"}}

subgraph Legend
  direction LR
  _p[Process] ~~~ _d{Decision} ~~~ _e{{External system}}
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Hard-block in issue-generation code/schema
  • ➕ Prevents the behavior even if the LLM ignores instructions
  • ➕ Deterministic enforcement (e.g., reject titles matching /^Evidence for #\d+/)
  • ➖ Requires code changes in the retro runner and potential deployment coordination
  • ➖ May need careful handling to avoid false positives (legit “evidence” wording)
2. Dedup heuristic: treat “Evidence for #XXXX” as duplicate automatically
  • ➕ Keeps pipeline fully automated while steering output to summary
  • ➕ Less intrusive than schema hard-rejection; can rewrite/redirect output
  • ➖ Still relies on parsing patterns; may miss variants
  • ➖ Adds maintenance burden for heuristics and edge cases
3. Post evidence as comments on the referenced issue (instead of PR summary)
  • ➕ Keeps evidence attached directly to the tracked issue for future triage
  • ➕ Reduces PR-comment verbosity for large retros
  • ➖ Requires authenticated write access and rate-limit handling
  • ➖ Higher risk of spam if retro runs are frequent; needs stronger safeguards

Recommendation: The PR’s approach (explicit instruction in the skill) is the fastest and lowest-risk mitigation for the immediate noise problem. However, consider a follow-up that adds a deterministic guardrail (schema/code-level rejection or automatic redirect to summary) to ensure the prohibition holds even when the model attempts prompt-workarounds.

Files changed (1) +3 / -1

Documentation (1) +3 / -1
SKILL.mdBan “Evidence for #XXXX” issue proposals; route evidence to summary +3/-1

Ban “Evidence for #XXXX” issue proposals; route evidence to summary

• Adds explicit guidance forbidding retro agents from filing “Evidence for #XXXX” issues as new proposals. Clarifies that corroborating evidence for existing issues must be captured in the 'summary' (posted as a PR comment) and that skipped duplicates should include the issue number plus the specific evidence found.

skills/retro-analysis/SKILL.md

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:22 PM UTC · Completed 2:34 PM UTC
Commit: 0b71d89 · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (1)

Context used
✅ Compliance rules (platform): 55 rules
✅ Skills: 4 invoked
  code-review
  code-implementation
  pr-review
  docs-review

Grey Divider


Action required

1. Protected skills/ file modified 📜 Skill insight § Compliance
Description
This PR modifies a protected governance/infrastructure path (skills/), which must not be
auto-approved and requires explicit human review. Even though the PR is linked to an authorizing
issue, the protected-path change still requires elevated scrutiny.
Code

skills/retro-analysis/SKILL.md[R124-126]

+**Do not file "evidence for" issues.** When your analysis produces evidence that supports or corroborates an existing open issue, put it in your `summary` field — not in a new proposal. Do not title proposals "Evidence for #XXXX" or use any other framing that makes a duplicate look like a new issue. The summary is posted as a comment on the originating PR, which preserves the data point. Filing evidence as a separate proposal creates noise that compounds across retro runs.
+
+When skipping, note the duplicate in your `summary` field — include the issue number and what specific evidence this retro found, so the human understands what was filtered and why.
Relevance

⭐⭐ Medium

No prior evidence that skills/ is treated as protected; repo-wide CODEOWNERS already enforces
human review.

PR-#27

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The compliance checklist marks any modifications under skills/ as protected-path changes that must
be surfaced as a finding and must not be auto-approved. The diff shows new/modified content within
skills/retro-analysis/SKILL.md.

skills/retro-analysis/SKILL.md[124-126]
Skill: pr-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR modifies a protected path (`skills/`), which must not be auto-approved and must receive explicit human review/approval.

## Issue Context
Protected governance/infrastructure paths require elevated scrutiny per compliance policy; this finding exists to ensure the PR is not merged via automated approval paths.

## Fix Focus Areas
- skills/retro-analysis/SKILL.md[124-126]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Wrong summary destination ✓ Resolved 🐞 Bug ≡ Correctness
Description
The new guidance says the summary is posted on the originating PR, but the retro workflow also
supports originating issues, so the comment can land on an issue thread as well. This mismatch can
mislead the agent about where its evidence will be preserved.
Code

skills/retro-analysis/SKILL.md[124]

+**Do not file "evidence for" issues.** When your analysis produces evidence that supports or corroborates an existing open issue, put it in your `summary` field — not in a new proposal. Do not title proposals "Evidence for #XXXX" or use any other framing that makes a duplicate look like a new issue. The summary is posted as a comment on the originating PR, which preserves the data point. Filing evidence as a separate proposal creates noise that compounds across retro runs.
Relevance

⭐⭐⭐ High

Repo script explicitly supports originating issues/PRs; docs should match behavior to avoid
misleading agents.

PR-#12

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
SKILL.md claims summaries are posted on the originating PR, but the implementation explicitly
accepts issue URLs and posts comments to the originating PR/issue number.

skills/retro-analysis/SKILL.md[124-126]
scripts/post-retro.sh[41-49]
scripts/post-retro.sh[125-150]
schemas/retro-result.schema.json[8-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The skill text states: “The summary is posted as a comment on the originating PR…”, but the retro pipeline supports originating URLs that are either PRs or issues. This makes the skill’s instruction incorrect for issue-triggered retros.

### Issue Context
The post script accepts `ORIGINATING_URL` matching both `/pull/N` and `/issues/N`, and posts the summary comment via the GitHub Issues comments endpoint (works for both PRs and issues).

### Fix Focus Areas
- skills/retro-analysis/SKILL.md[124-126]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Summary length overflow risk 🐞 Bug ☼ Reliability
Description
The new instruction routes corroborating evidence into the single summary field, which can exceed
the schema’s 16,384 character max on noisy retros and trigger schema-validation failures/retries.
This can reduce retro reliability by forcing additional iterations or failing if the agent can’t
compress enough.
Code

skills/retro-analysis/SKILL.md[R124-126]

+**Do not file "evidence for" issues.** When your analysis produces evidence that supports or corroborates an existing open issue, put it in your `summary` field — not in a new proposal. Do not title proposals "Evidence for #XXXX" or use any other framing that makes a duplicate look like a new issue. The summary is posted as a comment on the originating PR, which preserves the data point. Filing evidence as a separate proposal creates noise that compounds across retro runs.
+
+When skipping, note the duplicate in your `summary` field — include the issue number and what specific evidence this retro found, so the human understands what was filtered and why.
Relevance

⭐⭐ Medium

No historical evidence team flags summary maxLength risks; concern is plausible but not previously
discussed.

PR-#12

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The schema enforces a hard maxLength on summary, and the validation loop fails the run when schema
validation fails; the new guidance increases the chance of large summaries by moving many evidence
items into summary.

skills/retro-analysis/SKILL.md[124-126]
schemas/retro-result.schema.json[10-16]
scripts/validate-output-schema.sh[42-78]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The skill now instructs the agent to place evidence for existing issues into `summary`. Because `summary` is schema-limited (16,384 chars), a retro that accumulates many evidence items can exceed the limit and fail output validation.

### Issue Context
The retro harness validates output against `schemas/retro-result.schema.json` before post-processing. `summary` has `maxLength: 16384`.

### Fix Focus Areas
- skills/retro-analysis/SKILL.md[124-126]
- schemas/retro-result.schema.json[10-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread skills/retro-analysis/SKILL.md Outdated
Comment thread skills/retro-analysis/SKILL.md Outdated
Comment thread skills/retro-analysis/SKILL.md Outdated
@rh-hemartin rh-hemartin self-assigned this Jul 8, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] skills/retro-analysis/SKILL.md — This file is under the skills/ protected path. The PR links to Retro agent files "Evidence for" issues instead of skipping duplicates fullsend#3617 and provides clear rationale for the change (prohibiting the retro agent from filing "Evidence for" issues that bypass duplicate detection). Human approval is always required for protected-path changes, regardless of context.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 8, 2026

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. One minor note inline.

Comment thread skills/retro-analysis/SKILL.md Outdated
The retro agent's dedup check tells it to skip proposals covered by
existing issues, but the LLM invented a workaround: filing "Evidence
for #XXXX" issues that pass schema validation. This produced 336
noise issues in 3 days (69% of all retro output since July 5).

Add explicit prohibition to the retro-analysis skill. Evidence for
existing issues goes in the summary field (posted as a PR comment),
not as a new proposal.

Mirrors fullsend-ai/fullsend#3638.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
@rh-hemartin
rh-hemartin force-pushed the fix/3617-no-evidence-issues branch from 0b71d89 to 4cfc0f7 Compare July 8, 2026 14:43
Comment thread skills/retro-analysis/SKILL.md Outdated
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:44 PM UTC · Completed 2:51 PM UTC
Commit: 4cfc0f7 · View workflow run →

@rh-hemartin
rh-hemartin added this pull request to the merge queue Jul 8, 2026
Merged via the queue into main with commit 19a5984 Jul 8, 2026
13 of 14 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 8, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 2:49 PM UTC · Completed 2:54 PM UTC
Commit: 4cfc0f7 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already merged.

The /fs-review command only reviews open pull requests.

Posted by fullsend post-review check

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #59 — fix(retro): prohibit filing "evidence for" issues

Timeline: Human-authored PR by rh-hemartin, created and merged same day (2026-07-08, ~25 min from open to merge). Single-file change (+3/−1) to skills/retro-analysis/SKILL.md adding explicit prohibition against the retro agent filing "Evidence for #XXXX" proposals. This addressed an urgent operational issue: 336 of 485 retro-filed issues (69%) since July 5 were evidence-style duplicates that bypassed the skip-duplicates instruction.

Review quality analysis:

  • The fullsend review agent found only the protected-path issue (requiring human approval for skills/ changes). This is a valid but low-signal finding for a human-authored PR that already had the requires-manual-review label.
  • Qodo found 3 findings including a factual correctness bug (text says "originating PR" but pipeline also supports originating issues) rated HIGH relevance, and a summary overflow risk (routing all evidence into the 16,384-char summary field could cause schema validation failures).
  • Human reviewer ralphbean independently caught the same "PR or issue" wording bug and approved with that minor note.
  • Human reviewer ifireball suggested posting evidence as comments on the referenced issue (forward-looking feature), which led to issue feat(retro): post evidence comments on related issues #60.
  • The author (rh-hemartin) addressed ralphbean's feedback by amending the commit to say "originating PR or issue" before merging.

Key gap: The fullsend review agent missed a straightforward factual correctness issue that both qodo and a human reviewer caught. This is the same pattern as issue #46 (review agent missed issues: write permission gap, caught by qodo). Evidence supports existing issue #46 and upstream issues #3544 and #2199 — the review agent does not cross-reference factual claims in changed text against other parts of the codebase.

Existing tracking: Defense-in-depth title guard already tracked at fullsend-ai/fullsend#3641. Evidence-on-issue follow-up already tracked at #60. No existing issue covers the summary overflow risk introduced by this change.

Proposals filed

rh-hemartin pushed a commit to csoceanu/agents that referenced this pull request Jul 10, 2026
…limit

Add conciseness guidance to the retro-analysis skill for evidence
notes routed into the summary field: one sentence per existing
issue, prioritize most impactful evidence if space constrained.

Increase summary maxLength in retro-result.schema.json from
16,384 to 32,768 to provide additional headroom. GitHub supports
up to 65,536 chars in issue comments, so this is within platform
limits.

Both changes address the overflow risk introduced by PR fullsend-ai#59,
which routed all evidence for existing issues into summary
without length guidance or increased schema capacity.

Closes fullsend-ai#61
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retro agent files "Evidence for" issues instead of skipping duplicates

3 participants