Skip to content

fix(pr-management-triage): suppress already-acted-on PRs with unchanged head SHA - #1479

Open
liwenjie200543 wants to merge 1 commit into
apache:mainfrom
liwenjie200543:fix/triage-session-dedup
Open

liwenjie200543 wants to merge 1 commit into
apache:mainfrom
liwenjie200543:fix/triage-session-dedup

Conversation

@liwenjie200543

Copy link
Copy Markdown
Contributor

Summary

  • Implement the remaining acceptance criterion of Silent in-session de-dup of already-seen / already-skipped PRs #77: a PR already taken to a terminal action earlier in the same session is silently suppressed on re-encounter — no queue entry, no progress line, no prompt.
  • The suppression is head-SHA aware, per the resolution proposed on the issue: an entry whose cached head_sha matches the freshly fetched head SHA has nothing new to read and is dropped; a changed head SHA does not match the suppression and falls through to the existing staleness rule (drop entry, re-classify). The two rules are complementary rather than conflicting.
  • Extend the full-pagination loop pseudo-code, the Step 1 prose, the Session-cache invalidation section, and SKILL.md Step 1 accordingly; add three pagination-dedup regression cases (unchanged head suppressed / pushed head re-classified / cache entry without a terminal action_taken never suppresses) and restamp measured_tokens.

#1071 covered the pagination-level duplicate (a PR moving between pages); this patch covers the session-level one. The suppression set is session state and dies with the cache, so a fresh session re-surfaces everything.

Type of change

  • Skill change (skills/<name>/) — eval fixtures updated below
  • Tool / bridge contract (tools/<system>/*.md)
  • Python package
  • Documentation
  • CI / dev loop

Test plan

  • All 62 pr-management-triage eval fixtures assemble successfully in print mode (59 → 62: three new pagination-dedup cases)
  • skill-eval framework tests: 155 passed; the 19 failing tests are subprocess-spawning cases that fail identically on unmodified main in this Windows environment (WinError 2 spawning POSIX-style commands) — verified against a clean main worktree, byte-identical failure list, so no regression is introduced by this change
  • uv run --project tools/skill-token-count skill-token-count --write → measured_tokens: 5027
  • tools/dev/skill-surface-hash.py --fix reports surface_hash in sync (no headings added, moved, or renamed)
  • Spec-sync pre-check: tools/spec-loop/.last-sync (77884f5c) is a direct ancestor of the current tip, 16 commits behind; pr-management-family.md / triage-mode.md do not describe the pagination / session-cache layer, so no spec drift is introduced

Exact-automated (--cli) grading of the three new fixtures was not run locally (no model CLI in the authoring sandbox); the fixtures follow the existing pagination-dedup case shape and the expected outputs are pinned in expected.json.

RFC-AI-0004 compliance

  • Vendor neutrality — no provider-specific behavior added

Linked issues

Closes #77 — implements the resolution proposed in this comment: suppress silently only while the head SHA is unchanged, re-classify when it changed.

Generative AI disclosure

ZCode (GLM) was used while preparing this change. The contributor reviewed the diff and test results before submission.

…ed head SHA

Step 1 of the triage skill re-surfaced PRs the same session had already
taken to a terminal action, inviting a second, possibly different
decision. After pagination dedup, drop entries the session cache holds
under an action_taken whose head_sha equals the freshly fetched head
SHA; a changed head still re-classifies via the existing staleness
rule, making the two rules complementary per the resolution proposed
on the issue.

Generated-by: ZCode (GLM)
@github-actions github-actions Bot added the substrate:framework-dev Tool substrate: build / validate / eval the framework itself label Oct 1, 2026

@Kaap10 Kaap10 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 @liwenjie200543!

Went through the changes:

  • Step 1 session suppression logic is clean and properly distinguishes unchanged head_sha from fresh pushes.
  • Regression fixtures in pagination-dedup (cases 3–5) cover the edge cases nicely.
  • Eval suite counts and token measurements are kept in sync, and all CI checks pass.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

substrate:framework-dev Tool substrate: build / validate / eval the framework itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Silent in-session de-dup of already-seen / already-skipped PRs

2 participants