fix(CODEWIKI-009): CU-86akn96pk 4 review findings across 4 files

[flamingo]

Closes 4 review findings across 4 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

#	Fix confidence	Finding	Location
1	🟡 75 medium	test_clustering_proof.py uses print()/sys.exit() ad-hoc instead of the TestResults accumulator pattern	test_clustering_proof.py:80
2	🟡 75 medium	test_clustering_simple.py uses ad-hoc print/sys.exit instead of TestResults pattern	test_clustering_simple.py:99
3	🟡 70 medium	test_fqdn_normalization.py uses pytest/assert instead of the TestResults accumulator pattern	test_fqdn_normalization.py:1
4	🟢 90 high	test_clustering_local.py test function does not call results.add_test() for every failure path (e.g. exception branch inside try lacks per-assertion granularity) and uses raw sys.exit without full pass/fail bookkeeping for the API-key check	test_clustering_local.py:165

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

---

Run: https://product-hub.flamingo.so/admin/code-review
Run id: 9aa9712c-4ca2-4571-94bc-95e5cff4b3cd

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akn96pk CodeWiki review findings sweep (9 PRs)

[flamingo]

🦩 What this fix changed, finding by finding

4 finding(s) fixed in this draft — 4 explained inline on the diff.

[flamingo]

🦩 🟠 test_clustering_proof.py uses print()/sys.exit() ad-hoc instead of the TestResults accumulator pattern

Replaced the ad-hoc print()/sys.exit() pass/fail reporting at the bottom of the script with a local TestResults class (add_test()/print_summary()) mirroring the pattern in test_clustering_validation.py, and recorded the single clustering outcome (empty/one/multiple modules) as a test case via add_test(); the script now exits based on print_summary()'s aggregated result instead of branching sys.exit() calls. Risk: TestResults is defined locally in this file rather than imported from a shared module, since no such shared module currently exists in the repo to import from safely; if test_clustering_validation.py's TestResults is later extracted to a shared module, this file should be updated to import it instead of duplicating the class.

🤖 Prompt for AI agents

In test_clustering_proof.py around line 80, review and complete this code-review fix: test_clustering_proof.py uses print()/sys.exit() ad-hoc instead of the TestResults accumulator pattern.
What the draft fix changed: Replaced the ad-hoc print()/sys.exit() pass/fail reporting at the bottom of the script with a local TestResults class (add_test()/print_summary()) mirroring the pattern in test_clustering_validation.py, and recorded the single clustering outcome (empty/one/multiple modules) as a test case via add_test(); the script now exits based on print_summary()'s aggregated result instead of branching sys.exit() calls. Risk: TestResults is defined locally in this file rather than imported from a shared module, since no such shared module currently exists in the repo to import from safely; if test_clustering_validation.py's TestResults is later extracted to a shared module, this file should be updated to import it instead of duplicating the class.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 75 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 test_clustering_simple.py uses ad-hoc print/sys.exit instead of TestResults pattern

Replaced the ad-hoc print()/sys.exit() pass/fail reporting at the end of the script (previously if len(module_tree) == 0: ... sys.exit(1) else: ... sys.exit(0)) with a TestResults class defined in this file providing add_test() and print_summary(), matching the CODEWIKI-009 pattern. The script now records the clustering outcome via results.add_test(...) and determines the exit code from results.print_summary(). Risk: I could not see the actual shared TestResults implementation referenced by CODEWIKI-009 elsewhere in the repo, so I defined a local, self-contained class with the same interface (add_test, print_summary) rather than importing an unseen shared module, to avoid introducing a broken import.

🤖 Prompt for AI agents

In test_clustering_simple.py around line 99, review and complete this code-review fix: test_clustering_simple.py uses ad-hoc print/sys.exit instead of TestResults pattern.
What the draft fix changed: Replaced the ad-hoc print()/sys.exit() pass/fail reporting at the end of the script (previously `if len(module_tree) == 0: ... sys.exit(1) else: ... sys.exit(0)`) with a `TestResults` class defined in this file providing `add_test()` and `print_summary()`, matching the CODEWIKI-009 pattern. The script now records the clustering outcome via `results.add_test(...)` and determines the exit code from `results.print_summary()`. Risk: I could not see the actual shared `TestResults` implementation referenced by CODEWIKI-009 elsewhere in the repo, so I defined a local, self-contained class with the same interface (`add_test`, `print_summary`) rather than importing an unseen shared module, to avoid introducing a broken import.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 75 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 test_fqdn_normalization.py uses pytest/assert instead of the TestResults accumulator pattern

Rewrote test_fqdn_normalization.py to drop the pytest import and bare assert statements. Added a TestResults class with add_test() and print_summary() accumulator methods, converted every test_* function to accept a results parameter and record outcomes via results.add_test(name, condition, message) instead of raising via assert, and replaced the pytest.main(...) entrypoint in __main__ with explicit calls to each test function followed by results.print_summary() and a SystemExit based on overall success. This aggregates all failures into a structured summary instead of aborting on first failure, matching the described repo convention. Risk: the exact shape of TestResults/add_test/print_summary used elsewhere in the repo could differ slightly (e.g. additional methods, different signature ordering) since I could not see the canonical implementation — the reviewer should confirm this matches the established pattern used in other integration tests.

🤖 Prompt for AI agents

In test_fqdn_normalization.py around line 1, review and complete this code-review fix: test_fqdn_normalization.py uses pytest/assert instead of the TestResults accumulator pattern.
What the draft fix changed: Rewrote test_fqdn_normalization.py to drop the `pytest` import and bare `assert` statements. Added a `TestResults` class with `add_test()` and `print_summary()` accumulator methods, converted every `test_*` function to accept a `results` parameter and record outcomes via `results.add_test(name, condition, message)` instead of raising via `assert`, and replaced the `pytest.main(...)` entrypoint in `__main__` with explicit calls to each test function followed by `results.print_summary()` and a `SystemExit` based on overall success. This aggregates all failures into a structured summary instead of aborting on first failure, matching the described repo convention. Risk: the exact shape of `TestResults`/`add_test`/`print_summary` used elsewhere in the repo could differ slightly (e.g. additional methods, different signature ordering) since I could not see the canonical implementation — the reviewer should confirm this matches the established pattern used in other integration tests.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 70 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 test_clustering_local.py test function does not call results.add_test() for every failure path (e.g. exception branch inside try lacks per-assertion granularity) and uses raw sys.exit without full pass/fail bookkeeping for the API-key check

In the if __name__ == "__main__": block, test_results = TestResults() is now created before the API-key check; the missing-key branch now calls test_results.add_test("api_key_configured", False, ...) and test_results.print_summary() before sys.exit(1), and the success branch calls test_results.add_test("api_key_configured", True) so the bookkeeping and summary output reflect the API-key check in all paths.

🤖 Prompt for AI agents

In test_clustering_local.py around line 165, review and complete this code-review fix: test_clustering_local.py test function does not call results.add_test() for every failure path (e.g. exception branch inside try lacks per-assertion granularity) and uses raw sys.exit without full pass/fail bookkeeping for the API-key check.
What the draft fix changed: In the `if __name__ == "__main__":` block, `test_results = TestResults()` is now created before the API-key check; the missing-key branch now calls `test_results.add_test("api_key_configured", False, ...)` and `test_results.print_summary()` before `sys.exit(1)`, and the success branch calls `test_results.add_test("api_key_configured", True)` so the bookkeeping and summary output reflect the API-key check in all paths.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer