feat: Create TestRunner.Report with markdown generation

[andreasronge]

Summary

Creates a new TestRunner.Report module that extracts markdown report generation functionality from LispTestRunner. The Report module can now be shared by both JSON and Lisp test runners.

• Parameterizes report title by DSL name ("JSON", "Lisp", etc.)
• Reuses all formatting functions from TestRunner.Base (no duplication)
• Generates structured markdown with summary, results table, failed details, and all programs
• Includes comprehensive documentation with @moduledoc, @doc with examples, and @SPEC

Test plan

• Code compiles with mix compile --warnings-as-errors
• All tests pass with mix test
• Credo analysis passes
• Manual verification of Report.generate output structure

Fixes #198

[github-actions]

PR Review: Create TestRunner.Report with markdown generation

Summary

This PR creates a new PtcDemo.TestRunner.Report module that extracts markdown report generation from LispTestRunner into a shared module. The implementation is well-structured and follows the refactoring plan closely.

What's Good

• Clean extraction: The report generation logic is properly extracted with the DSL name parameterized as required
• Proper reuse of Base functions: Uses TestRunner.Base functions (format_cost, format_duration, truncate, format_attempt_result) - no duplication
• Comprehensive documentation: Excellent @moduledoc, @doc with examples, and @spec for all public functions
• Handles edge cases: Gracefully handles empty all_programs lists and missing :program keys
• Code matches spec: The implementation follows the structure defined in test-runner-refactor-plan.md (lines 172-188)

Issues (Must Fix)

1. Potential division by zero - report.ex:67
   • Problem: Float.round(summary.total_attempts / summary.total, 2) will raise ArithmeticError if summary.total is 0 (empty results list)
   • Impact: Report generation will crash for empty test runs
   • Fix: Add a guard or use a conditional:

     avg_attempts = if summary.total > 0, do: Float.round(summary.total_attempts / summary.total, 2), else: 0.0

   • Complexity: Mechanical (one-line change)

Suggestions (Optional)

None - the implementation is focused and appropriately scoped for Phase 1.2 of the refactoring plan.

Security

No concerns. The module only generates markdown strings from structured data with no external input handling.

Documentation

No updates needed. The module includes comprehensive inline documentation. The refactoring plan (demo/docs/test-runner-refactor-plan.md) already documents the planned module structure, and this implementation matches it.

Verdict

Request Changes - The division by zero issue should be addressed before merge. This is a one-line fix.

[andreasronge]

@claude please fix these issues from PR review:

Issue 1: Division by zero in average attempts calculation

Location: demo/lib/ptc_demo/test_runner/report.ex:69
Problem: Float.round(summary.total_attempts / summary.total, 2) will raise ArithmeticError if summary.total is 0 (empty results list), causing report generation to crash for empty test runs.
Fix: Add a guard to handle the case when summary.total is 0:

| Avg Attempts/Test | #{if summary.total > 0, do: Float.round(summary.total_attempts / summary.total, 2), else: 0.0} |

Please fix this issue in a SINGLE commit. Make minimal changes to resolve the issue.

[andreasronge]

Auto-Triage Summary

Decisions Made

#	Issue	Decision	Action
1	Division by zero in average attempts calculation (report.ex:69)	FIX_NOW	Included in fix request above

Status

• FIX_NOW items: 1 (batched in single fix comment above)
• Issues created: 0
• Items ignored: 0

Reasoning

The division by zero issue is a straightforward bug in the newly created Report module that will cause crashes for empty test runs. This is:

• ✅ In-scope: Bug in the code added by this PR
• ✅ Mechanical: One-line conditional guard fix
• ✅ Simple: No design decisions needed

The fix adds a guard to return 0.0 when summary.total is 0, preventing the ArithmeticError while maintaining correct behavior for non-empty results.

[andreasronge]

Fix Summary

Issue 1: Division by zero in average attempts calculation ✅

Status: Fixed

Change: Added conditional guard on line 69 of

The fix prevents when calculating average attempts for empty test results by:

• Checking if summary.total > 0 before dividing
• Returning 0.0 when no tests were run

Verification:

• ✅ Code formatted
• ✅ Compilation successful (no warnings)
• ✅ Credo linting passed
• ✅ All tests pass

The minimal change resolves the crash when generating reports for empty test runs.

[github-actions]

PR Review: Create TestRunner.Report with markdown generation

Summary

Creates a shared TestRunner.Report module that extracts markdown report generation from LispTestRunner, parameterized by DSL name. The implementation is clean, well-documented, and correctly reuses TestRunner.Base formatting functions.

What's Good

• Proper reuse: Uses TestRunner.Base functions (format_duration, format_cost, truncate, format_attempt_result) instead of duplicating the formatting logic that exists in LispTestRunner lines 652-675
• Comprehensive documentation: @moduledoc, @doc with examples, and @spec for all public functions
• Clean extraction: Report generation logic faithfully mirrors the LispTestRunner implementation (lines 679-798)
• Edge case handling: Handles empty results with guard if summary.total > 0 for average calculation
• DSL parameterization: Title correctly uses dsl_name parameter ("PTC-#{dsl_name} Test Report")

Issues (Must Fix)

None. The implementation is correct and matches the requirements.

Suggestions (Optional)

1. Function naming differs from spec - Nice-to-have

   • Observation: The test-runner-refactor-plan.md spec shows generate_report/2 and write_report/3, but the implementation uses generate/2 and write/3
   • Assessment: The shorter names are arguably cleaner and follow Elixir conventions better (module name already provides context)
   • Recommendation: No action needed - the deviation is an improvement. Consider updating the spec document to match the implementation in a future PR if maintaining spec accuracy matters.

2. Private helpers could be documented - Nice-to-have

   • The private functions (generate_results_table, generate_failed_details, etc.) lack documentation
   • Assessment: For private functions in a focused module, this is acceptable
   • Recommendation: No action needed

Security

No concerns. The module generates markdown reports from internal data structures - no user input handling or external data ingestion.

Documentation

Documentation is excellent:

• @moduledoc explains purpose, report structure, and what sections are included
• @doc on both public functions includes parameter descriptions and examples
• @spec types are accurate

Verdict

Approve - Clean implementation that correctly extracts report generation into a shared module. No bugs, good documentation, proper code reuse. Ready to merge.

[andreasronge]

Auto-Triage Summary (Cycle 1)

Review Status

The latest review (post-fix) resulted in Approve with no required changes.

Decisions Made

#	Issue	Decision	Rationale
1	Function naming differs from spec	IGNORE	Reviewer explicitly marked "No action needed" - shorter names follow Elixir conventions better
2	Private helpers could be documented	IGNORE	Reviewer explicitly marked "No action needed" - acceptable for private functions in focused module

Status

• FIX_NOW items: 0
• Issues created: 0
• Items ignored: 2 (both explicitly optional per reviewer)

Next Steps

✅ Added ready-to-merge label - auto-merge will handle merging once all checks pass.