Repository navigation
fix(level7): targeted behavioral fixes for M04/M06/M07 checkers + P03 CLI - #67
Merged
Merged
Conversation
- M04 check.py: the checker only verified that POST /tournaments returned 202 with a job_id, so a fake that ran the whole tournament synchronously and fabricated a "completed" job entry passed every gate — defeating the mission's entire point. Added a behavioral proof: monkeypatch SimulationService.simulate_tournament to block on a threading.Event, fire POST in a helper thread, and require the HTTP response to come back while the worker is still blocked. A synchronous implementation now hangs the helper thread and is correctly rejected; cleanup (Event release + thread join + method restore) runs in a finally block. - M06 check.py: the checker only grepped "ThreadPoolExecutor" in the function's source text, so creating the pool and never calling submit()/map() on it (falling back to the sequential baseline) passed. Added a recording ThreadPoolExecutor stand-in that runs work inline (keeping results correct) while recording every independent item handed to submit()/map(); requires >=2 independent items were actually dispatched. Verified against map-based and submit-based correct solutions (both pass) and the create-but-unused fake (fails).
check.py previously only grepped "ProcessPoolExecutor" in the function's source text, so three distinct fakes all passed: a pool created but never used (sequential fallback), a single submit() for the entire tournament, and a worker that ignores its seed and returns a constant result. - Gate A: a recording ProcessPoolExecutor stand-in (same technique as M06) proves the pool actually receives >1 independent work item and rejects lambdas submitted to it. Handles both pool.map and chunked pool.submit styles without requiring an exact item count. - New exact-match check: _simulate_one(seed) is fully deterministic, so a correct parallel run over seeds 0..N-1 must produce the exact same hero_wins as the sequential baseline — not just "a number that adds up to N". This is what catches the constant-result worker; Gate A alone does not. - Gate B: a real (unpatched) ProcessPoolExecutor smoke test runs the actual _simulate_one across real worker processes under this platform's spawn start method, plus a pickle.dumps() check — proving the worker is genuinely spawn-safe, not just textually present. Verified against the map-based reference solution, a submit-based variant, a chunked variant (multiple seeds per work item), and all four fakes (unused pool, single-submit, lambda worker, constant-result worker) — each fake fails at the gate it's supposed to. docs: moved the full simulate_tournament_parallel implementation out of the main README flow into a collapsible hint — this was the only mission where students write ProcessPool code themselves, so the full answer was the highest-leakage spot in the level. Main flow now has the contract, pickling constraint, and a syntax reminder instead of a ready solution.
cli.py's `start` command called worker.submit(job_id, lambda: ...), but ProcessPoolTournamentWorker.submit() takes (job_id, battles: int) — the old callable-based Worker contract was removed everywhere else but never updated here. Reproduced the exact failure: `start` printed a job_id but the job stayed stuck at "running" forever (confirmed via the real JSON job file, error: null). Fixing just the lambda->int argument was not sufficient: submit() spawns a daemon thread, and the CLI process exits right after start() returns, killing that thread before it can do any work — reproduced this independently with a minimal int-only submit() call, same stuck-forever result. A short-lived CLI invocation cannot host a background job that outlives it without real daemonization (Celery/RQ-style), which is out of scope here. Chosen fix: `start` now waits for the job to reach a terminal status before the process exits, using the same JobRepository polling pattern taught elsewhere in the course, then prints the result. The job is still driven through the real ProcessPoolTournamentWorker/JobRepository machinery — `status`/`report` run afterward in separate processes still read the same persisted, completed job from disk. Also added `battles <= 0` validation, since the CLI bypasses the API's Pydantic layer entirely and previously "succeeded" silently with a meaningless 0-battle result. Verified end-to-end across three separate process invocations (start, then status, then report in fresh processes) and the error path (battles=0 -> controlled exit 1, not a hung job). No lambdas remain in cli.py. ProcessPoolTournamentWorker itself was not modified.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First targeted-rework batch for Level 7, scoped to exactly 5 items from the full-level audit. No architecture changes —
ProcessPoolTournamentWorker(module-level worker, serializable seed data,pool.map, localRandom, spawn compatibility) was verified correct and left untouched.check.py: replaced the "POST returns 202" check (which a fully-synchronous fake job also satisfies) with a behavioral proof — blocksimulate_tournamenton athreading.Event, fire the POST from a helper thread, and require the HTTP response to come back while the worker is still blocked.check.py: replaced the"ThreadPoolExecutor" in sourcegrep with a recordingThreadPoolExecutorstand-in that provessubmit()/map()was actually called with ≥2 independent work items, not just instantiated.check.py: same recording-executor technique forProcessPoolExecutor, plus a new exact-match check (_simulate_oneis deterministic by seed, so parallel and sequential must produce identicalhero_wins) and a real, unpatchedProcessPoolExecutorspawn/pickling smoke test.README.md: moved the fullsimulate_tournament_parallelimplementation out of the main flow into a collapsible hint — this is the only mission where students write ProcessPool code themselves.cli.py: fixed the stale callable-basedworker.submit(job_id, lambda: ...)call (current contract issubmit(job_id, battles: int)), and — after confirming experimentally that the lambda→int fix alone still leaves jobs stuck in"running"forever (daemon thread dies with the short-lived CLI process) — madestartwait for the job to reach a terminal status before exiting, using the sameJobRepositorypolling pattern taught elsewhere in the course. Also addedbattles <= 0validation, since the CLI bypasses the API's Pydantic layer entirely.Explicitly not touched
M08, M01, M05, the P03 Ruff gate,
.gitignorefor job files, NumPy/Pandas, course-wide integration — reserved for Batch 2.Test plan
pool.mapsolution → PASS;pool.submitsolution → PASS; pool-created-but-unused fake → FAILrunningforever, confirmed via raw JSON); lambda→int alone insufficient (reproduced independently); full fix verified end-to-end across three separate process invocations (start, thenstatus, thenreport);battles=0→ controlled error, exit 1tools/author_check.py→ 727 checks passed, 0 errorstools/course_status.py→ runs clean__pycache__, no leaked job files, working tree clean except the intended patch🤖 Generated with Claude Code