Skip to content

Instantly share code, notes, and snippets.

@happysundar
Created September 14, 2026 02:27
Show Gist options
  • Select an option

  • Save happysundar/28f51bd04eb26e405bb4cd85b73ff1d2 to your computer and use it in GitHub Desktop.

Select an option

Save happysundar/28f51bd04eb26e405bb4cd85b73ff1d2 to your computer and use it in GitHub Desktop.
Ticket 04 thermonuclear code review and fixes

Ticket 04 thermonuclear code review and fixes

Ticket 04 thermonuclear code review and fixes

Initial review

Verdict: reject.

  1. Blocker: incomplete AST-prefix execution. _execution_prefix drops completed attribute assignments (candidate_params.py:174-183). box.value = 1 is skipped, so a later candidate using it fails. Execute every completed non-Expr statement, while only rounding up candidate-binding assignments.

  2. Blocker: incomplete LDM module restoration. sys.modules is restored, but ebay_pydec.ldm remains bound to the evaluation module on the parent package (candidate_params.py:221-263). Snapshot and restore that package attribute too.

  3. Blocker: escaped SystemExit. value = sys.exit(7) bypasses evaluation_failed (candidate_params.py:243) and exits with 7. Wrap SystemExit alongside ordinary evaluation exceptions.

  4. Major: order-dependent legacy-path validation. /production/prod/flow.dec resolves as prod, despite the ticket requiring paths containing legacy names to fail. Validate legacy segments before selecting a canonical environment.

The CLI/library boundary is clean, AST compilation and stdout redirection are correct, and no ticket 05-09 fields leaked in.

Review verification: 71 tests passed, Ruff clean, focused coverage 98%. Pyright was unavailable to the reviewer.

Fixes made

Each finding received a regression test that failed before the fix and passed afterward.

  1. Completed non-Expr assignments now execute when their full statement ends at or before the requested line. Only candidate-binding assignments round up when the requested line falls inside the statement.
  2. Evaluation snapshots and restores both the ebay_pydec.ldm* entries in sys.modules and the ebay_pydec.ldm parent-package attribute.
  3. SystemExit is converted to evaluation_failed, preserving the DEC source line and message instead of terminating the command.
  4. Every path component is checked for legacy environment spellings before canonical environment selection or explicit flag handling.

Follow-up review

Verdict: approve. No remaining presumptive blocker or material maintainability gap in ticket 04.

Optional test hardening noted by the reviewer: cover LDM evaluation when ebay_pydec.ldm was initially absent, then assert the imported parent attribute is removed afterward.

Follow-up verification: 77 tests pass; Ruff and formatting pass.

Final verification

  • uv run pytest: 77 passed
  • uv run ruff check .: passed
  • uv run ruff format --check .: 14 files already formatted
  • Ticket-04 evaluator Pyright check: 0 errors
  • mog ci test-python: passed
  • mog ci coverage-gate --language python: passed; 85.629% versus 85.550% target, +0.080 percentage points
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment