[CHORE] Add Passed unit Test

[HeshamHM28]

PR Type

Enhancement, Tests

---

Description

• Add passed tests list to PR comment

• Expose helper to collect passed test names

• Extend JSON schema with passed tests field

---

Diagram Walkthrough

flowchart LR
  A["TestResults.get_passed_existing_test_names"] -- "collects names from loop 1" --> B["List[str] of passed tests"]
  C["PrComment.to_json"] -- "calls get_passed_existing_test_names" --> D["adds passed_existing_tests to JSON"]

Loading

File Walkthrough

	Relevant files
Enhancement	PrComment.py Add passed tests list to PR comment JSON                                  codeflash/github/PrComment.py Extend JSON return type to include list[str]. Add "passed_existing_tests" to output using TestResults helper. +3/-2
Tests	models.py Helper to collect names of passed existing tests                  codeflash/models/models.py Implement get_passed_existing_test_names on TestResults. Filter by first loop, passed, and existing unit tests. Format names as module::Class.test or module::test. +18/-0

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Type Hint Consistency The return type of to_json now includes list[str] due to the new passed_existing_tests field. Ensure all downstream consumers and JSON schema updates align with this expanded type and that serialization/deserialization handles lists of strings correctly. def to_json(self) -> dict[str, Union[str, int, dict[str, dict[str, int]], list[BenchmarkDetail], list[str], None]]: report_table = { test_type.to_name(): result for test_type, result in self.winning_behavior_test_results.get_test_pass_fail_report_by_type().items() if test_type.to_name() } result: dict[str, Union[str, int, dict[str, dict[str, int]], list[BenchmarkDetail], list[str], None]] = { "optimization_explanation": self.optimization_explanation, Ordering/Determinism get_passed_existing_test_names accumulates names in iteration order of self.test_results. If consumers expect stable ordering, consider sorting results (e.g., by module/class/test name) before returning to avoid flaky diffs/comments. def get_passed_existing_test_names(self) -> list[str]: """Get a list of passed existing unit test names. Only considers tests from the first loop (loop_index == 1) to avoid duplicates. Returns test names in the format: module_path::ClassName.test_name or module_path::test_name """ passed_tests: list[str] = [] for test_result in self.test_results: if ( test_result.loop_index == 1 and test_result.did_pass and test_result.test_type == TestType.EXISTING_UNIT_TEST ): class_prefix = f"{test_result.id.test_class_name}." if test_result.id.test_class_name else "" test_name = f"{test_result.id.test_module_path}::{class_prefix}{test_result.id.test_function_name}" passed_tests.append(test_name) return passed_tests

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard optional benchmarking access Avoid invoking number_of_loops() on a potentially None winning_benchmarking_test_results. This will raise at runtime when benchmarking data is absent. Guard the access and emit a sensible default (e.g., 0) to keep JSON generation robust. codeflash/github/PrComment.py [42-46] def to_json(self) -> dict[str, Union[str, int, dict[str, dict[str, int]], list[BenchmarkDetail], list[str], None]]: report_table = { test_type.to_name(): result for test_type, result in self.winning_behavior_test_results.get_test_pass_fail_report_by_type().items() if test_type.to_name() } + + loop_count = self.winning_benchmarking_test_results.number_of_loops() if self.winning_benchmarking_test_results else 0 result: dict[str, Union[str, int, dict[str, dict[str, int]], list[BenchmarkDetail], list[str], None]] = { "optimization_explanation": self.optimization_explanation, "best_runtime": humanize_runtime(self.best_runtime), "original_runtime": humanize_runtime(self.original_runtime), "function_name": self.function_name, "file_path": self.relative_file_path, "speedup_x": self.speedup_x, "speedup_pct": self.speedup_pct, - "loop_count": self.winning_benchmarking_test_results.number_of_loops(), + "loop_count": loop_count, "report_table": report_table, "passed_existing_tests": self.winning_behavior_test_results.get_passed_existing_test_names(), "benchmark_details": self.benchmark_details if self.benchmark_details else None, } Suggestion importance[1-10]: 7 __ Why: Adding a safe guard for winning_benchmarking_test_results prevents a potential runtime error if it's None and cleanly defaults loop_count, improving robustness without altering behavior otherwise.	Medium
	Correct loop index filtering Ensure the loop-index filtering matches the actual 0/1-based indexing semantics used elsewhere. If the first loop is 0-based, the current loop_index == 1 will miss all first-iteration results. Align the condition to include the true first pass to avoid returning an empty or incomplete list. codeflash/models/models.py [689-705] def get_passed_existing_test_names(self) -> list[str]: """Get a list of passed existing unit test names. - Only considers tests from the first loop (loop_index == 1) to avoid duplicates. + Considers only the first loop to avoid duplicates. Returns test names in the format: module_path::ClassName.test_name or module_path::test_name """ passed_tests: list[str] = [] for test_result in self.test_results: + is_first_loop = test_result.loop_index in (0, 1) if ( - test_result.loop_index == 1 + is_first_loop and test_result.did_pass and test_result.test_type == TestType.EXISTING_UNIT_TEST ): class_prefix = f"{test_result.id.test_class_name}." if test_result.id.test_class_name else "" test_name = f"{test_result.id.test_module_path}::{class_prefix}{test_result.id.test_function_name}" passed_tests.append(test_name) return passed_tests Suggestion importance[1-10]: 5 __ Why: The concern about 0/1-based loop indexing is plausible, but the PR explicitly documents using loop_index == 1 as the first loop; changing it to include 0 may be unnecessary or wrong without broader context. Moderate impact if indexing is indeed 0-based elsewhere.	Low