feat: add PR body generator from agent bundle#207
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a script and associated tests to generate a deterministic Markdown pull request body from an agent artifact bundle. The feedback focuses on improving the robustness of JSON parsing by enforcing strict type checking for list fields—specifically validation_evidence, problems, and changed_files—recommending that a RuntimeError be raised for non-list types instead of using silent fallbacks.
| if not isinstance(validation_evidence, list) or not validation_evidence: | ||
| return ["- No validation evidence provided in bundle."] |
There was a problem hiding this comment.
The current implementation performs a silent fallback if validation_evidence is not a list. According to the general rules, non-list types for expected list fields should raise a RuntimeError to maintain strictness, while null or missing values should be treated as empty lists.
| if not isinstance(validation_evidence, list) or not validation_evidence: | |
| return ["- No validation evidence provided in bundle."] | |
| if validation_evidence is not None and not isinstance(validation_evidence, list): | |
| raise RuntimeError("validation_evidence must be a list") | |
| if not validation_evidence: | |
| return ["- No validation evidence provided in bundle."] |
References
- When processing JSON data, treat null or missing values for expected list fields as empty lists, but raise a RuntimeError for other non-list types to maintain strictness.
| problems = safe_pr_gate.get("problems") | ||
| if isinstance(problems, list) and problems: | ||
| lines.append("- problems:") | ||
| lines.extend(f" - `{problem}`" for problem in problems if isinstance(problem, str)) | ||
| else: | ||
| lines.append("- problems: `none`") |
There was a problem hiding this comment.
The problems field should follow the strictness rule for expected list fields. If the field is present but is not a list, it should raise a RuntimeError instead of silently falling back to the default message. Additionally, use type guards for items within the list to prevent crashes on malformed data.
| problems = safe_pr_gate.get("problems") | |
| if isinstance(problems, list) and problems: | |
| lines.append("- problems:") | |
| lines.extend(f" - `{problem}`" for problem in problems if isinstance(problem, str)) | |
| else: | |
| lines.append("- problems: `none`") | |
| problems = safe_pr_gate.get("problems") | |
| if problems is not None and not isinstance(problems, list): | |
| raise RuntimeError("safe_pr_gate.problems must be a list") | |
| if problems: | |
| lines.append("- problems:") | |
| lines.extend(f" - {problem}" for problem in problems if isinstance(problem, str)) | |
| else: | |
| lines.append("- problems: none") |
References
- When processing JSON data, treat null or missing values for expected list fields as empty lists, but raise a RuntimeError for other non-list types to maintain strictness.
- Use type guards like isinstance(item, dict) before accessing fields within list items to prevent crashes on malformed data.
| changed_files = bundle.get("changed_files") | ||
| changed_file_lines = _bullet_list(changed_files if isinstance(changed_files, list) else [], "No changed files provided in bundle.") |
There was a problem hiding this comment.
The changed_files field should also be handled strictly. If it is present but not a list, a RuntimeError should be raised per the repository's general rules.
| changed_files = bundle.get("changed_files") | |
| changed_file_lines = _bullet_list(changed_files if isinstance(changed_files, list) else [], "No changed files provided in bundle.") | |
| changed_files = bundle.get("changed_files") | |
| if changed_files is not None and not isinstance(changed_files, list): | |
| raise RuntimeError("changed_files must be a list") | |
| changed_file_lines = _bullet_list(changed_files or [], "No changed files provided in bundle.") |
References
- When processing JSON data, treat null or missing values for expected list fields as empty lists, but raise a RuntimeError for other non-list types to maintain strictness.
Adds a deterministic Markdown PR body generator from agent artifact bundles.
Validation: