Skip to content

fix(#369): cap evidence issues at 5 per parent - #417

Closed
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/369-cap-evidence-issues
Closed

fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/369-cap-evidence-issues

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown

Add an evidence issue cap to the retro-analysis skill. Before filing an evidence proposal for a parent issue, the agent now queries the count of open evidence issues with matching titles. If the count is >= 5, the agent skips filing and notes the data point in its summary comment instead.

This prevents unbounded evidence accumulation for well-documented patterns, reducing noise in the issue tracker without losing information.


Closes #369

Post-script verification

  • Branch is not main/master (agent/369-cap-evidence-issues)
  • Secret scan passed (gitleaks — aa6585ddb3dff17dccfd4dba8b6c5c306275156a..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add an evidence issue cap to the retro-analysis skill. Before
filing an evidence proposal for a parent issue, the agent now
queries the count of open evidence issues with matching titles.
If the count is >= 5, the agent skips filing and notes the data
point in its summary comment instead.

This prevents unbounded evidence accumulation for
well-documented patterns, reducing noise in the issue tracker
without losing information.

Closes #369
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:56 AM UTC · Completed 12:03 PM UTC
Commit: aa6585d · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [API contract / query correctness] internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md:139 — The gh api query uses in:title with a quoted phrase, which matches substrings. An issue titled e.g. Revert: Evidence for #42 would also match, potentially inflating the count. Given the cap is advisory (concurrency note already accepts off-by-one overshoot) and the title convention is defined by this skill itself, false-positive matches are extremely unlikely in practice.

  • [instruction ambiguity / ordering] internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md:126 — The instructions do not explicitly state whether the evidence cap check should run before or after the broader duplicate search. Running the cap check first (one API call) could save the cost of a subagent dedup search when the cap is already reached.
    Remediation: Add a sentence like: "For evidence proposals, run the cap check before the broader duplicate search — if the cap is already reached, you can skip both the evidence proposal and its dedup search."

Previous run

Review

Verdict: Approve

The PR adds an evidence issue cap section to the retro-analysis skill, directly implementing the behavior requested in #369. The implementation is well-structured: it provides a concrete gh api query for counting existing evidence issues, specifies fail-closed behavior when the query fails, and documents the concurrency limitation honestly. The change is correctly scoped to the skill file (not the agent definition), follows established patterns in the codebase, and introduces no security concerns.

Findings

Medium — Concurrency note understates maximum overshoot (internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md)

The concurrency note states two concurrent retro runs can exceed the cap "by one." This is correct for the two-run scenario described, but N>2 concurrent runs could all observe the same count, exceeding the cap by N-1. For example, three runs seeing count=4 would all file, producing 7 total (overshoot=2). Given the cap is explicitly advisory and concurrent retro runs are rare, this is acceptable — but the note could say "by a small amount" rather than "by one" to be precise. Non-blocking.

Low — Heading level asymmetry (internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md)

The new ### Evidence issue cap is a subsection of ## Before proposing: check for existing issues, but the existing duplicate-check content under the same H2 has no heading of its own. This creates a mild structural asymmetry. The placement is defensible — both are pre-filing checks — but restructuring the H2 to have two explicit H3 subsections (### Duplicate check and ### Evidence issue cap) would be cleaner.

Low — Title convention relies on prompt compliance (internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md)

The Evidence for #N title format is introduced as a convention but isn't enforced by the output schema or post-retro script. The cap check's accuracy depends entirely on the agent consistently using this exact title. A future enhancement could add title-format validation in the post-script, but for now prompt-level enforcement is reasonable.

Low — User-facing retro docs don't explain evidence filing (docs/agents/retro.md)

The user-facing retro agent documentation doesn't explain the evidence filing pattern or the new cap. If users see evidence issues in their tracker, documenting the behavior and its limits would reduce confusion. Consider a brief addition in a follow-up.


Labels: PR modifies retro-analysis skill documentation with behavioral guidance

Previous run (2)

Review

Findings

Medium

  • [incorrect claim about runtime mechanism] internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md:170 — The concurrency note states "the concurrency group in retro.yml limits the window" for the evidence cap race condition. However, retro.yml's concurrency group is scoped per triggering PR/issue (fullsend-retro-{source_repo}-{pr_or_issue_number}), not per parent issue. Two retro runs from different PRs filing evidence for the same parent issue have different concurrency groups and run fully in parallel. The stated mitigation does not function for the described race.
    Remediation: Remove the incorrect claim about the concurrency group. Replace with: "Concurrency note: Two retro runs triggered by different PRs can both query the count before either files, causing both to see the same value and potentially exceeding the cap by one. This is an accepted limitation — the cap is advisory and a brief overshoot does not meaningfully harm the issue tracker."

Low

  • [undocumented convention dependency] internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md:129 — The evidence cap check relies on proposals having titles matching Evidence for #N, but this title convention is not defined or enforced anywhere in the skill file or the retro agent definition. The cap mechanism silently does nothing if the agent uses a different title format.

  • [code-organization] internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md:135 — Bash code block formatting is inconsistent with the established pattern in the same file. The existing gh api command at line 111 places the URL on a separate continuation line, while the new command places the URL on the same line as gh api.


Labels: PR modifies skill documentation in internal/scaffold

Previous run (3)

Review — approve

PR: #417fix(#369): cap evidence issues at 5 per parent
Scope: internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md (+48 lines)

Well-scoped change that adds an evidence issue cap to the retro-analysis skill, matching what #369 requested. The fail-closed design (skip filing when the count query fails) is the right call. The concurrency caveat is honest and the accepted overshoot-by-one is reasonable given the retro.yml concurrency group.

Checked dimensions

Dimension Result
Correctness ✅ Query logic is sound; fail-closed on error is correct
Security ✅ Read-only search query, no mutation, no secrets
Intent & scope ✅ Matches #369 — issue authorizes either agent def or skill file
Style ✅ Follows existing SKILL.md conventions (H3, code blocks, bold)
Docs currency ✅ No stale references introduced
Cross-repo contracts ⏭ Skipped — no exported APIs modified

Minor observations (non-blocking)

  1. Placeholder clarity in query template — The code block shows %23N (URL-encoded #N). The accompanying text says "Replace N with the parent issue number," which is correct, but an LLM executing this could conceivably replace the entire %23N token rather than just N. Consider showing the unencoded form alongside (e.g., # i.e., "Evidence for #42") to anchor intent.

  2. Section connectivity — The new "Evidence issue cap" section is a peer H3 to the duplicate-check section above it. An agent processing the skill linearly would benefit from a brief cross-reference at the end of the duplicate-check section (e.g., "After deduplication, also apply the evidence cap below"). Minor — the linear ordering already implies sequence.

  3. Commit prefixfix(#369) is borderline. COMMITS.md allows fix for prompt adjustments that correct a defect, and unbounded evidence accumulation is reasonably framed as a defect. refactor(#369) would also be defensible. Not blocking.

No medium or higher findings. Approving.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jul 12, 2026
@guyoron1

Copy link
Copy Markdown
Owner

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:08 PM UTC · Completed 1:19 PM UTC
Commit: aa6585d · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment documentation Improvements or additions to documentation and removed ready-for-merge All reviewers approved — ready to merge labels Jul 12, 2026
@guyoron1

Copy link
Copy Markdown
Owner

/fs-fix

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 12, 2026

Copy link
Copy Markdown
Author

🤖 Finished Fix · ✅ Success · Started 1:43 PM UTC · Completed 1:51 PM UTC
Commit: aa6585d · View workflow run →

- Remove incorrect concurrency group claim in evidence cap section;
  retro.yml groups are per-PR, not per-parent-issue
- Document the Evidence for #N title convention explicitly
- Reformat gh api code block to match existing continuation-line style

Addresses review feedback on #417
@fullsend-ai-coder

Copy link
Copy Markdown
Author

🔧 Fix agent — iteration 1 (human-triggered)

Addressed all 3 review findings: removed incorrect concurrency group claim (medium), documented the Evidence for #N title convention (low), and reformatted the code block to match existing style (low).

Fixed (3):

  1. incorrect claim about runtime mechanism (internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md): Removed incorrect claim that retro.yml concurrency group limits the evidence cap race window. The concurrency group is scoped per triggering PR/issue, not per parent issue — two retro runs from different PRs targeting the same parent run in parallel. Replaced with accurate statement that the cap is advisory and a brief overshoot is accepted.
  2. undocumented convention dependency (internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md): Added explicit documentation of the Evidence for #N title convention at the top of the evidence cap section, establishing it as a required format before it is used in the query logic.
  3. code-organization: bash code block formatting inconsistency (internal/scaffold/fullsend-repo/skills/retro-analysis/SKILL.md): Reformatted the gh api code block to place the URL on a continuation line, matching the established pattern at line 111.

Tests: passed

Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 12, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:58 PM UTC · Completed 2:07 PM UTC
Commit: aa6585d · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Jul 12, 2026
@guyoron1

Copy link
Copy Markdown
Owner

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:19 AM UTC · Completed 6:30 AM UTC
Commit: e49ecd5 · View workflow run →

Dispatch a subagent to count existing evidence issues:

```bash
gh api \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] API contract / query correctness

The gh api query uses in:title with a quoted phrase, which matches substrings. An issue titled e.g. 'Revert: Evidence for #42' would also match, potentially inflating the count. Given the cap is advisory and the title convention is defined by this skill itself, false-positive matches are extremely unlikely in practice.


When skipping, note the duplicate in your `summary` field so the human understands what was filtered and why.

### Evidence issue cap

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] instruction ambiguity / ordering

The instructions do not explicitly state whether the evidence cap check should run before or after the broader duplicate search. Running the cap check first (one API call) could save the cost of a subagent dedup search when the cap is already reached.

Suggested fix: Add a sentence like: 'For evidence proposals, run the cap check before the broader duplicate search — if the cap is already reached, you can skip both the evidence proposal and its dedup search.'

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed ready-for-merge All reviewers approved — ready to merge labels Jul 13, 2026
@guyoron1 guyoron1 closed this Jul 14, 2026
@guyoron1
guyoron1 deleted the agent/369-cap-evidence-issues branch July 29, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ARCHIVED] Benchmark issue 369

1 participant