fix(skills): prevent /review from spamming @greptileai when already approved

[carlos-alm]

Summary

• Fixes the /review skill blindly re-triggering @greptileai even when Greptile already approved (thumbsup + no inline comments), as seen on PR refactor: address SLOC warnings in domain and features layers #621
• Replaces the single emoji-on-reply check with a proper decision matrix that checks the trigger comment's thumbsup reaction, inline comment count, and elapsed time
• Adds a 15-minute wait rule: if Greptile hasn't responded yet, wait instead of posting duplicate triggers

Test plan

• Run /review on a PR where Greptile already gave thumbsup + no comments — verify it skips re-triggering
• Run /review on a PR with outstanding Greptile inline comments — verify it addresses them and re-triggers
• Run /review on a PR where @greptileai was just posted (<15 min ago) — verify it waits instead of posting again

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review Analysis

Reviewing PR #628: fix(skills): prevent /review from spamming @greptileai when already approved

Todo List

• Read modified skill file to understand changes
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity
• Phase 4: Code Quality
• Phase 5: Backlog Compliance Assessment
• Provide final rating and recommendation

---

Root Cause Analysis

Problem Being Solved: The /review skill was blindly re-triggering @greptileai even when Greptile had already approved a PR (indicated by thumbsup reaction + no new inline comments), leading to spam and unnecessary reviewer pings.

Why This Approach: Replaces a simplistic "check reactions on your last reply" method with a sophisticated decision matrix that:

• Finds the most recent @greptileai trigger comment (from anyone)
• Checks for thumbsup reactions from greptile-apps[bot]
• Counts inline comments posted after the trigger
• Implements a 15-minute wait rule to prevent duplicate triggers

Risk Assessment: LOW RISK. This is a pure logic improvement to an automation workflow with no impact on core functionality.

Backlog Compliance

• Zero-dep: ✓ — No new dependencies, uses existing gh CLI and jq
• Foundation-aligned: ✓ — Improves automation efficiency without violating core principles
• Problem-fit: 2 — Nice workflow improvement, but doesn't address the core "lost AI" problem
• Breaking: No — Pure internal skill improvement with no API changes
• Tier: 1

Critical Concerns

None identified. The implementation is well-structured and addresses the root cause properly.

Technical Analysis

✅ Strengths:

1. Sophisticated decision logic: Implements proper state tracking with multiple conditions rather than naive emoji checking
2. Time-based protection: 15-minute wait rule prevents race conditions and duplicate triggers
3. Correct API usage: Uses proper GitHub API endpoints (/comments, /reactions) with appropriate filtering
4. Clear documentation: Decision matrix is well-explained with explicit logic branches
5. Preserves existing behavior: Only changes the Greptile re-trigger logic, maintains all other functionality

✅ Code Quality:

• Bash scripting: Proper use of jq for JSON parsing and GitHub CLI for API calls
• Error handling: Appropriate use of pagination (--paginate) for complete data retrieval
• Variable naming: Clear variable names (trigger_comment, trigger_id, trigger_time)
• Logic flow: Well-structured conditional branches with clear exit conditions

✅ No Config/Test Integrity Issues:

• No configuration files modified
• No test expectations lowered
• No linting rules disabled
• This is pure workflow logic enhancement

🔍 Minor Observations:

1. The old logic was checking reactions on "your last reply" vs. new logic checks reactions on "last @greptileai trigger" — this is actually more correct since triggers can come from anyone
2. New logic is more complex but necessarily so to handle edge cases properly
3. The 15-minute wait rule is a reasonable heuristic to balance responsiveness vs. spam prevention

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: This is a well-implemented fix for a real workflow issue. The approach is sound, the logic is comprehensive, and there are no technical concerns. The only reason it's not 5 stars is that it's an incremental improvement rather than addressing core codegraph functionality, but within its scope it's excellent work.

The decision matrix properly handles all the edge cases mentioned in the PR description and should eliminate the spam behavior while maintaining proper reviewer workflow.

---

| Branch: fix/review-greptile-retrigger

[greptile-apps]

Greptile Summary

This PR replaces the simple "check for a positive emoji on your last reply to Greptile" heuristic with a proper multi-step decision matrix. The new logic counts prior @greptileai trigger comments (excluding Greptile's own responses), finds the most recent one, queries its reactions and any new inline comments posted after it, computes elapsed time portably via python3, and then routes to one of six explicit branches. All four issues surfaced in previous review rounds — missing decision-matrix branches, the null-ID first-run bug, Greptile's own responses polluting the trigger list, and the absence of a concrete elapsed-time calculation — are addressed.

Key changes:

• Added a trigger_count guard for the first-ever-run case, bypassing both check 1 and check 2 and triggering immediately
• Both the count query and the last-trigger query now include select(.user.login != "greptile-apps[bot]") to exclude Greptile's own comments
• Elapsed minutes are computed with python3 (ISO 8601-aware, cross-platform) rather than being left to the agent's inference
• Decision matrix expanded to 6 branches covering all meaningful combinations of {thumbsup, inline comments, elapsed time}, including the two previously missing ones ("thumbsup + inline comments", "no thumbsup + >15 min + no inline")
• Updated the Rules summary at the bottom to match the new wait/check logic

Confidence Score: 5/5

Safe to merge — all four previously raised issues are addressed and no new issues are introduced.

Every concern from prior review rounds is now fixed: the two missing decision-matrix branches are present, the first-run null-ID path has an upfront guard, Greptile's own comments are excluded from the trigger filter, and elapsed time is computed concretely with python3. The six decision branches cover all meaningful states, the bash snippets are portable, and the Rules footer is consistent with the new logic.

No files require special attention.

Important Files Changed

Filename	Overview
.claude/skills/review/SKILL.md	Decision matrix expanded, first-run null guard added, Greptile self-comment filter added, portable elapsed-time computation added — all prior issues resolved, logic is consistent and complete.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Run /review on a PR] --> B{trigger_count == 0?}
    B -- Yes --> C[First-ever run\nPost @greptileai immediately\nProceed to step 2h]
    B -- No --> D[Fetch last trigger comment\ntrigger_id, trigger_time]
    D --> E[Query thumbsup reaction\nQuery new inline comments\nCompute elapsed_minutes]
    E --> F{Thumbsup exists?}
    F -- Yes --> G{New inline comments?}
    G -- Yes --> H[Address inline comments\nthen re-trigger]
    G -- No --> I{elapsed_minutes > 15?}
    I -- Yes --> J[Skip re-triggering\nGreptile approved]
    I -- No --> K[Wait until 15 min\nthen re-check]
    F -- No --> L{elapsed_minutes > 15?}
    L -- No --> M[Wait until 15 min\nthen re-check]
    L -- Yes --> N{New inline comments?}
    N -- Yes --> O[Address inline comments\nthen re-trigger]
    N -- No --> P[Re-trigger once\nGreptile may have missed it]
    H --> Q[Check 2: Were Greptile\ncomments addressed?]
    O --> Q
    P --> Q
    Q -- No changes in response --> R[Skip re-triggering]
    Q -- Changes made --> S[Post @greptileai]

Loading

_{Reviews (3): Last reviewed commit: "fix(skills): handle first-ever @greptile..." | Re-trigger Greptile}

[greptile-apps]

Decision matrix has two unhandled cases

The four listed cases leave gaps that will leave the agent with no instruction:

1. No thumbsup AND >15 min AND no new inline comments — Greptile never responded at all (no reaction, no inline comments) after the full wait. The matrix says nothing about this state. The agent will likely stall or make an arbitrary choice. The correct behavior here is probably to re-trigger (Greptile may have missed the notification).

2. Thumbsup exists AND new inline comments exist (regardless of time) — Greptile approved but also left inline comments. This is a contradictory-but-real state (e.g., Greptile leaves nit comments but still thumbsups). The matrix doesn't say whether to treat the thumbsup or the inline comments as the authoritative signal.

Suggested additions to the decision logic:

- No thumbsup AND >15 min AND no new inline comments → **Greptile never responded. Re-trigger once** (it may have missed the notification).
- Thumbsup exists AND new inline comments exist → **Treat inline comments as outstanding concerns. Address them (step 2e), then re-trigger.**

[carlos-alm]

Fixed. Added both missing branches to the decision matrix:

• Thumbsup + inline comments → Treat inline comments as outstanding concerns. Address them, then re-trigger. (Greptile sometimes thumbsups but still leaves nit comments.)
• No thumbsup + >15 min + no inline comments → Greptile never responded. Re-trigger once (it may have missed the notification).

[greptile-apps]

No guard when no prior @greptileai trigger exists

[...] | last on an empty array returns null in jq. If a PR has never had an @greptileai comment (first run), trigger_id and trigger_time are both the string "null", causing the subsequent reactions API call to hit /issues/comments/null/reactions (a 404) and the inline-comments filter to compare timestamps against the literal string "null".

The agent should detect the empty case upfront and skip the matrix entirely (jumping straight to check 2 / re-trigger):

trigger_count=$(gh api repos/optave/codegraph/issues/<number>/comments --paginate \
  --jq '[.[] | select(.body | test("@greptileai"))] | length')
if [ "$trigger_count" -eq 0 ]; then
  # No prior trigger — skip approval check, proceed to check 2
  ...
fi

[carlos-alm]

Fixed. Added a \ check upfront — if no prior \ trigger exists (excluding Greptile's own responses), the script skips the approval check entirely and proceeds to check 2 (whether we addressed any Greptile feedback). This avoids the null ID/timestamp issue.

[greptile-apps]

test("@greptileai") will match Greptile's own response comments

Greptile's review responses frequently contain the phrase @greptileai in their body (e.g. "I reviewed this as requested via @greptileai"). The select(.body | test("@greptileai")) filter picks up those Greptile-authored comments too, so | last may resolve to a Greptile response rather than the human trigger comment. This would cause the reactions and inline-comment checks to be run against the wrong comment ID/timestamp.

The filter should exclude Greptile's own comments:

trigger_comment=$(gh api repos/optave/codegraph/issues/<number>/comments --paginate \
  --jq '[.[] | select(.body | test("@greptileai")) | select(.user.login != "greptile-apps[bot]")] | last | {id: .id, created_at: .created_at}')

[carlos-alm]

Fixed. The jq filter now excludes Greptile's own comments with select(.user.login != "greptile-apps[bot]") in both the count query and the last-trigger query, so | last will only resolve to an actual human trigger comment.

[greptile-apps]

No tooling provided for the elapsed-time check

The decision branches on >15 min vs <15 min since the trigger, but the bash block gives the agent no command to compute elapsed time. The agent receives $trigger_time (an ISO 8601 string) and must infer "now" on its own — which is reliable only if the agent's clock/date context is accurate.

A concrete command removes ambiguity:

# Elapsed time in seconds since the trigger
elapsed_seconds=$(( $(date -u +%s) - $(date -u -d "$trigger_time" +%s 2>/dev/null || date -u -j -f "%Y-%m-%dT%H:%M:%SZ" "$trigger_time" +%s) ))
elapsed_minutes=$(( elapsed_seconds / 60 ))
echo "Minutes since trigger: $elapsed_minutes"

(The date -d form works on Linux; the -j -f form works on macOS — providing both or using python3 -c makes it portable.)

[carlos-alm]

Fixed. Added concrete elapsed-time computation using python3 for ISO 8601 parsing (portable across platforms) and date -u +%s for current epoch. The result is stored in $elapsed_minutes which the decision logic references directly.

[carlos-alm]

@greptileai

[carlos-alm]

Fixed the remaining concern about the first-ever trigger case. When trigger_count is 0, the script now posts @greptileai directly and proceeds to the wait-and-recheck loop (step 2h), bypassing Check 2 entirely. This ensures the first-ever trigger always fires on a fresh PR.

[carlos-alm]

Fixed the remaining concern about the first-ever trigger case. When trigger_count is 0, the script now posts @greptileai directly and proceeds to the wait-and-recheck loop (step 2h), bypassing Check 2 entirely. This ensures the first-ever trigger always fires on a fresh PR.

[carlos-alm]

@greptileai