Greptile Summary

Pure rename of a local counter from curr_num_correction_steps to parse_attempts in both ModelFacade.generate and ModelFacade.agenerate, with a clarifying comment added near the declaration in generate and a back-reference in agenerate. No control-flow, behavior, or public API changes.

• Renames the local variable in both sync and async paths, including the initialization, increment, conditional check, and restart-reset sites — all six touch-points are updated consistently.
• Adds a comment in generate accurately describing that parse_attempts counts attempts (1-indexed), so parse_attempts <= max_correction_steps permits exactly max_correction_steps corrections after the initial attempt; agenerate cross-references this comment.

Important Files Changed

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[LLM response received] --> B[parse_attempts += 1]
    B --> C{parser succeeds?}
    C -- yes --> D[return output]
    C -- no --> E{max_correction_steps == 0\nAND max_conversation_restarts == 0?}
    E -- yes --> F[raise immediately]
    E -- no --> G{parse_attempts <= max_correction_steps?}
    G -- yes --> H[append error as user message\nloop back to LLM]
    G -- no --> I{curr_num_restarts < max_conversation_restarts?}
    I -- yes --> J[parse_attempts = 0\ncurr_num_restarts += 1\nrestore restart checkpoint]
    I -- no --> K[raise GenerationValidationError]
    H --> A
    J --> A

_{Reviews (2): Last reviewed commit: "Merge branch 'main' into fix/371-rename-..." | Re-trigger Greptile}

PR #627 Review — chore(engine): rename correction-step counter for clarity

Summary

Pure local-variable rename in ModelFacade.generate and ModelFacade.agenerate: curr_num_correction_steps → parse_attempts. Adds a clarifying comment in the sync path and a back-reference comment in the async path. No behavior change; public API (max_correction_steps, max_conversation_restarts) and the error message text are untouched. Closes #371.

Findings

Correctness

• Rename is complete and consistent. Verified via grep: zero remaining curr_num_correction_steps references repo-wide, and all six occurrences in both generate and agenerate are renamed symmetrically (declaration, increment, bound check, restart reset).
• Control flow is unchanged — no reordering of the increment vs. the <= max_correction_steps check, no change to the max_correction_steps == 0 and max_conversation_restarts == 0 short-circuit, no change to the restart-reset path. The PR author explicitly rejected the alternative of moving the increment into except, with a correct rationale (would change behavior at max_correction_steps == 0 and interact with the restart path).
• The new name accurately describes the semantics. The counter is incremented before each parse attempt, so with max_correction_steps=1 the sequence is: attempt 1 → parse_attempts=1, 1 <= 1 True → inject correction → attempt 2 → parse_attempts=2, 2 <= 1 False → fall through. That matches "initial + max_correction_steps corrections" as the comment claims.
• The existing error message "Unsuccessful generation despite {max_correction_steps} correction steps…" still reads correctly because it references the public config value, not the internal counter.

Code quality & conventions

• Follows AGENTS.md invariants: typed code unchanged, from __future__ import annotations already present, absolute imports preserved.
• The comment is appropriately narrow — it explains the why (why the bound check reads as it does given pre-increment), which is exactly what the style guide prefers over restating the what.
• The back-reference in agenerate ("See generate for a description…") is a reasonable DRY choice, though if generate is ever deleted or split the pointer rots silently. Minor — not worth changing.

Risk / blast radius

• Minimal. Local variable only; not exposed in any public surface, not used in any serialization format, not logged. Blast radius is confined to the two methods.
• Scope creep: none. The PR resists the temptation to also move the increment or rename the public parameter, which is the right call for a chore/clarity PR.

Tests

• No new tests required — behavior is unchanged. PR description confirms the full engine suite (1975 tests) still passes, including the parametrized correction-step tests in test_facade.py.
• No test file references the renamed variable (it was always local), so nothing had to move.

Nits (optional, non-blocking)

• The comment phrase "permits exactly max_correction_steps corrections after the initial attempt" is accurate but a bit dense. A reader stepping through the first time may find it easier as: "the initial attempt is counted as attempt 1, so the bound permits max_correction_steps subsequent correction attempts." Matter of taste.
• The two occurrences of parse_attempts = 0 (declaration + restart reset) are easy to miss if someone later refactors — consider a tiny helper or a named constant later, but not in this PR.

Verdict

Approve — merge as-is. Small, well-motivated, correctly-scoped cleanup. The rename closes the readability gap flagged in #371 without changing behavior, and the author chose the safer of the two possible fixes (rename vs. restructure) with clear reasoning. No issues found.