Skip to content

refactor(review_manager): iterator-based ReviewSessionManager to replace queue[0] + skip_card pattern [post-F82] #41

Description

@ImmortalDemonGod

What

The current ReviewSessionManager design exposes a mutable review_queue list and requires callers to manually advance past processed cards via skip_card() (added by the F82 fix at review_manager.py:155-158) or rely on the success path calling _remove_card_from_queue() internally (review_manager.py:210). This split-responsibility design is fragile: the F82 bug was directly caused by the caller failing to remove a failed card. An iterator or generator protocol would make advancement automatic and eliminate the need for callers to know about queue internals.

Why deferred

Explicitly classified as nice-to-have / architectural in both .aiv/plans/c2-f82-plan.md §6 OUT OF SCOPE and the PR completion contract's OUT-OF-SCOPE REMINDERS:

"ReviewSessionManager iterator refactor — architectural; out of scope by design."

The F82 fix required the minimum-surface change (public skip_card wrapper over the existing private method). A full iterator refactor would touch ReviewSessionManager, review_ui.py, and all related tests, and warrants its own design review and PR.

Acceptance criteria

  • ReviewSessionManager provides an iterator protocol (__iter__/__next__) or a cards() generator that yields each card and automatically removes it from the active set after the caller signals completion or failure
  • review_ui.py iterates without explicit skip_card or _remove_card_from_queue calls
  • Verify skip_card callers post-refactor: grep -rn "skip_card" flashcore/ tests/ should confirm no orphaned call sites
  • All existing test_review_ui.py tests pass without the closure-based _skip mock pattern; queue advancement is covered by the iterator contract
  • test_review_manager.py covers the iterator protocol, the no-repeat guarantee, and the unknown-UUID no-op case

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions