Skip to content

demo: retry-after parser (greploop e2e v2) - #77

Closed
Kyzcreig wants to merge 2 commits into
mainfrom
apollo/greploop-e2e-v2
Closed

Kyzcreig wants to merge 2 commits into
mainfrom
apollo/greploop-e2e-v2

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

Greploop live e2e (paid Greptile). Deliberately sub-4 helper to exercise: Greptile <4 -> VETO -> auto-dispatch fix -> fix -> re-review -> converge. Opened READY (not draft) so Greptile reviews it. Closed after the loop runs.

@github-actions

github-actions Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

🔎 Lint report: apollo/greploop-e2e-v2 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11254 on HEAD, 11254 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5878 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@greptile-apps

greptile-apps Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR replaces the deliberately-broken v1 parse_retry_after helper with a corrected implementation that addresses all three issues flagged in the prior review round. The function now returns None (not 0) on parse failure, catches only (ValueError, TypeError) instead of a bare except, and uses no mutable default argument cache.

  • Return contract fixed: the docstring and implementation now explicitly return None for unparseable input, letting callers distinguish a genuine zero-delay from a parse error.
  • Exception handling narrowed: the except (ValueError, TypeError) clause no longer silently swallows KeyboardInterrupt, SystemExit, or GeneratorExit.
  • Unit suffix parsing: h, m, and s suffixes are handled with correct multipliers (3600, 60, 1); plain numerics fall through to a bare float() call.

Confidence Score: 5/5

Safe to merge — all previously identified issues have been corrected and no new defects are introduced.

The helper is a single pure function with no side-effects, no I/O, and no shared state. The three issues from the prior review round (broad exception clause, ambiguous zero return on failure, mutable default cache) are all fully resolved in this version.

No files require special attention.

Important Files Changed

Filename Overview
tools/greploop_e2e_demo.py New file: corrected retry-after parser; all three prior review issues resolved — bare except narrowed, return value changed from 0 to None on failure, mutable default cache removed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["parse_retry_after(value)"] --> B{value is None?}
    B -- Yes --> C[return None]
    B -- No --> D["s = str(value).strip().lower()"]
    D --> E{s is empty?}
    E -- Yes --> C
    E -- No --> F{try block}
    F --> G{s.endswith 'h'?}
    G -- Yes --> H["return float(s[:-1]) * 3600"]
    G -- No --> I{s.endswith 'm'?}
    I -- Yes --> J["return float(s[:-1]) * 60"]
    I -- No --> K{s.endswith 's'?}
    K -- Yes --> L["return float(s[:-1])"]
    K -- No --> M["return float(s)"]
    F -- "ValueError / TypeError" --> C
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["parse_retry_after(value)"] --> B{value is None?}
    B -- Yes --> C[return None]
    B -- No --> D["s = str(value).strip().lower()"]
    D --> E{s is empty?}
    E -- Yes --> C
    E -- No --> F{try block}
    F --> G{s.endswith 'h'?}
    G -- Yes --> H["return float(s[:-1]) * 3600"]
    G -- No --> I{s.endswith 'm'?}
    I -- Yes --> J["return float(s[:-1]) * 60"]
    I -- No --> K{s.endswith 's'?}
    K -- Yes --> L["return float(s[:-1])"]
    K -- No --> M["return float(s)"]
    F -- "ValueError / TypeError" --> C
Loading

Reviews (2): Last reviewed commit: "fix: address Greptile findings (no bare ..." | Re-trigger Greptile

Comment thread tools/greploop_e2e_demo.py Outdated
Comment thread tools/greploop_e2e_demo.py Outdated
Comment thread tools/greploop_e2e_demo.py Outdated
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 greploop auto-remediation → @apollo

This PR was VETO'd by the greploop decision engine. Fix the findings below on this branch; Greptile will re-review on push.

OBJECTIVE: address the Greptile findings on kyzcreig/hermes-agent#77 so it re-reviews >=4 with zero unresolved P0/P1, floor green.
OWNER: apollo  (sha f303fa30, branch via PR head)
DECISION: VETO — ['deterministic floor not present-and-green: floor:ci=RED, floor:sast=ABSENT(=red,fail-closed)']

BLOCKING FINDINGS (2 P0/P1):
  [P1] tools/greploop_e2e_demo.py:21 — Bare `except:` swallows `KeyboardInterrupt`, `SystemExit`, and `GeneratorExit` in addition to expected parsing errors. Any keyboard interrupt issued while this function is executing would be silently consumed and the call would return `0` i
  [P1] tools/greploop_e2e_demo.py:21 — On parse failure the function returns `0`, which is indistinguishable from a legitimately parsed `"0s"` or integer `0` input. Any caller using the return value to gate a retry delay will silently proceed with no delay on an unrecognised for

SCOPE: ONLY these findings. Smallest diff. No opportunistic refactor.
PUSH to the SAME PR branch (re-triggers Greptile). Run the full floor locally first.
RETURN: end with a ```json structured-handoff `code` envelope {task_id,status,summary,evidence,result:{tests_passed,files_changed,pr_number,regen_clean}}.

@Kyzcreig Kyzcreig added the greploop:fixing greploop auto-remediation in progress label Jun 22, 2026
…, drop mutable-default cache)

Greploop auto-remediation on #77:
- bare except -> except (ValueError, TypeError) (no longer swallows KeyboardInterrupt/SystemExit)
- return None on parse failure (distinguishable from a legit 0; callers can't silently skip a delay)
- removed the mutable-default-arg cache anti-pattern
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 greploop auto-remediation → @apollo

This PR was VETO'd by the greploop decision engine. Fix the findings below on this branch; Greptile will re-review on push.

OBJECTIVE: address the Greptile findings on kyzcreig/hermes-agent#77 so it re-reviews >=4 with zero unresolved P0/P1, floor green.
OWNER: apollo  (sha 780f8021, branch via PR head)
DECISION: VETO — ['deterministic floor not present-and-green: floor:ci=RED, floor:secret_scan=ABSENT(=red,fail-closed)']

BLOCKING FINDINGS (2 P0/P1):
  [P1] tools/greploop_e2e_demo.py:21 — Bare `except:` swallows `KeyboardInterrupt`, `SystemExit`, and `GeneratorExit` in addition to expected parsing errors. Any keyboard interrupt issued while this function is executing would be silently consumed and the call would return `0` i
  [P1] tools/greploop_e2e_demo.py:21 — On parse failure the function returns `0`, which is indistinguishable from a legitimately parsed `"0s"` or integer `0` input. Any caller using the return value to gate a retry delay will silently proceed with no delay on an unrecognised for

SCOPE: ONLY these findings. Smallest diff. No opportunistic refactor.
PUSH to the SAME PR branch (re-triggers Greptile). Run the full floor locally first.
RETURN: end with a ```json structured-handoff `code` envelope {task_id,status,summary,evidence,result:{tests_passed,files_changed,pr_number,regen_clean}}.

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

Labels

greploop:fixing greploop auto-remediation in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant