Skip to content

feat(e3): unify review packet construction paths#228

Open
agustif wants to merge 1 commit intopeteromallet:mainfrom
agustif:feat/e3-review-packet-unification
Open

feat(e3): unify review packet construction paths#228
agustif wants to merge 1 commit intopeteromallet:mainfrom
agustif:feat/e3-review-packet-unification

Conversation

@agustif
Copy link

@agustif agustif commented Mar 4, 2026

Summary

Implements Epic #209 and sub-issues #219 #220 #221.

This refactor removes duplicated review packet assembly logic in prepare, --run-batches, and --external-start by routing all three through the shared coordinator/builder contract.

What Changed

  • Added canonical prepare next-command builder in packet layer:
    • desloppify/app/commands/review/packet/build.py
    • new build_prepare_next_command(...)
  • Refactored prepare to use shared coordinator packet path:
    • desloppify/app/commands/review/prepare.py
    • now calls resolve_review_packet_context(...) + build_review_packet_payload(...)
    • preserves zero-file hint behavior and raises CommandError with existing guidance
  • Refactored run-batches packet preparation to shared coordinator:
    • desloppify/app/commands/review/batch/orchestrator.py
    • _load_or_prepare_packet(...) now uses shared context/builder and explicit PacketValidationError wrapping
    • keeps legacy blind packet path behavior via blind_path_override=_blind_packet_path()
  • Refactored external-start packet preparation to shared coordinator:
    • desloppify/app/commands/review/external.py
    • _prepare_packet_snapshot(...) now uses shared context/builder
    • now includes the same config redaction + file-cap policy path as other modes
    • keeps legacy blind packet path behavior via blind_path_override=_blind_packet_path()
  • Added regression tests for parity/delegation:
    • desloppify/tests/review/policy/test_packet_mode_parity.py
    • asserts shared packet fields are equal across prepare/batch/external for same inputs
    • asserts all modes delegate packet assembly to shared coordinator builder

Engineering Outcomes

  • One canonical packet-construction seam for all review modes.
  • Shared policy application (max_files_per_batch, config redaction, retrospective options) now converges through one path.
  • Mode-specific behavior remains explicit only in next_command shaping.
  • Reduced drift risk and reduced maintenance surface for future review packet changes.

Verification

  • Lint:
    • source .venv/bin/activate && ruff check desloppify/app/commands/review/prepare.py desloppify/app/commands/review/batch/orchestrator.py desloppify/app/commands/review/external.py desloppify/tests/review/policy/test_packet_mode_parity.py
  • Tests:
    • source .venv/bin/activate && pytest -q desloppify/tests/review/review_commands_cases.py desloppify/tests/review/import_scoring/test_review_external.py desloppify/tests/commands/review/test_review_process_guards_direct.py desloppify/tests/review/policy/test_packet_mode_parity.py
    • Result: 103 passed

Tracking

Parent program: #206

@agustif agustif force-pushed the feat/e3-review-packet-unification branch from cd91f96 to 2858eda Compare March 4, 2026 22:39
@agustif agustif marked this pull request as ready for review March 4, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant