nvidia--skillspector/contrib/batch_scan/tests/docs/BUGS_FOUND.md

Production Code Bugs Found & Fixed

Covers three phases: 6/23 (API pool refactor) + 6/24-25 (test architecture) + 6/26 (upstream merge + review hardening) All discovered by tests or test-driven audits

---

🔴 Production Code Bugs (15)

6/23 — Discovered During API Pool Refactor

#	Location	Bug	Symptom	Fix	Discovery Method
B1	api_pool.py:snapshot()	Deadlock — self._lock is not reentrant. snapshot() calls self.active_requests property while holding the lock → property internally acquires the same lock again	Process hangs	Read fields directly within the locked region, do not call property	Integration test
B2	api_pool.py:_capacity_summary()	Deadlock — Same as above. acquire() calls self.total_capacity property while holding the lock	Same as above	Same as above	Integration test
B3	api_pool.py:PooledChatModel._ainvoke_with_retry()	Async event loop blocking — acquire() synchronously blocks on Condition.wait(), asyncio event loop stalls	Concurrent performance degradation	Added try_acquire() non-blocking fast path	Integration test
B4	api_pool.py:record_retry_success()	Counting error — Increments on retry attempt, not retry success	Report data is misleading	Moved to after llm.invoke() succeeds, inside if attempt > 0 condition	Code review
B5	api_pool.py:set_api_pool(None)	Does not restore original function — After calling set_api_pool(None), the patched wrapper remains in memory	Subsequent calls still use the old path	Save _original_get_chat_model, restore when None	Integration test
B6	runner.py:Patch 6	Pydantic alias dependency — Only sets kwargs["timeout"], relying on Pydantic v2 alias to cover the canonical name	May break on upstream Pydantic version upgrade	Set both kwargs["timeout"] + kwargs["request_timeout"]	Audit discovery
B7	runner.py:cleanup_result()	Unreachable code — shutil.rmtree(ignore_errors=True) never raises, subprocess rm -rf fallback never executes	Dead code	Removed fallback branch + unused import	Code review
B8	runner.py:Patch 2/3	Overly broad exception handling — except (json.JSONDecodeError, Exception) makes JSONDecodeError redundant under Exception, and masks the difference between Pydantic validation errors and JSON parse errors	Masks real bugs	Split into separate except json.JSONDecodeError (LLM output quality issue) and except Exception (upstream schema change), with logs distinguishing "invalid JSON" vs "schema validation failed"	Code review
B9	batch_scan.py:main()	Report delay — with ThreadPoolExecutor calls shutdown(wait=True) on exit, waiting for stuck worker threads. Timed-out skipped skills are still running, blocking report output	Report waits 80-100s	Changed to executor.shutdown(wait=False), do not wait for dead threads	Integration test

6/24-25 — Discovered During Test Architecture Audit

#	Location	Bug	Symptom	Fix	Discovery Method
B10	runner.py:_apply_patches()	Nested premature restore — _patches_active: bool flag. Inner __exit__ removes patches that the outer block is still using	Patches silently deactivate	Changed to _patches_depth: int nesting counter	Code review + nesting test
B11	test_runner_patches.py:TestSetupFunction.tearDownClass	Infinite loop — from runner import _patches_depth copies the int value. while _patches_depth > 0: reads the local copy, which is never 0	Test process hangs permanently	Changed to import runner as _r; while _r._patches_depth > 0:	Random-order test
B12	test_runner_patches.py:test_setup_applies_patches	False assertion — assertIsNot(init, LLMAnalyzerBase.__init__ if False else True) is always True	Test always passes, cannot detect patch failure	Changed to save orig_init reference then assertIsNot(init, orig_init)	Audit discovery
B13	runner.py:_check_signature()	Does not detect parameter kind — Only checks parameter name existence, not whether it is keyword-only. If upstream changes to def __init__(self, *, base_prompt, model), the check still passes	Patch may crash on newer Python 3 versions	Added KEYWORD_ONLY detection, raises RuntimeError when found	Audit discovery
B14	runner.py:_original_chatopenai_init	Capture timing depends on import order — Captured when _apply_patches() runs. If another module pre-modifies ChatOpenAI.__init__, the wrong version is captured	Test environment may be incorrect	Moved to module load time (captured on import runner.py)	Audit discovery
B15	test_runner_patches.py:Patch 4/5	Missing functional verification — Only checks that method references are replaced, does not verify that the replacement actually appends JSON instructions	Patch 4/5 failure is undetectable	Added 2 functional tests: assertIn("Respond with ONLY a JSON object", prompt)	Mutation testing

6/26 — Discovered During Upstream Merge + Reviewer Response

#	Location	Bug	Symptom	Fix	Discovery Method
B16	runner.py:set_api_pool()	Pool bypass: graph path — Only patched llm_utils.get_chat_model. llm_analyzer_base imports via from ... import, creating a local reference. Graph analyzers (95% LLM calls) called the unpatched local reference. snapshot()['rate_limits_hit'] always 0.	Pool appears wired but graph path bypasses it entirely	Added _llm_analyzer_base.get_chat_model = _pooled_get_chat_model; test_pool_wiring.py now verifies LLMAnalyzerBase._llm is PooledChatModel	PR re-review after upstream merge

---

🟡 Test Code Bugs (3)

#	Location	Bug	Fix
T1	test_api_pool.py:test_exponential_backoff_values	Tests the math formula min(30*2^(n-1), 300), not the pool's actual release(success=False) behavior	Changed to go through the real release path
T2	test_api_pool.py:_make_key()	Dead code — defined but never called	Removed
T3	test_gap_fill.py:_VALID_FINDING	Module-level mutable dict — shared state risk	Changed to _valid_finding(**overrides) factory function

---

📊 Statistics

Category	Count
Production code bugs (fixed)	16
Test code bugs (fixed)	3
Known blind spots (accepted)	4 (Q13, Q16, Q17, Q18)
Mutation MISSED (not production bugs)	9