ADR-253: Add ReviewComment/ReviewThread dataclasses, extend PipelineContext, add parse_json_list_output

[jodavis-claude]

Work item: ADR-253

Add the pure data-model and serialization layer that the subsequent pipeline-step changes in ADR-254 depend on: ReviewComment/ReviewThread dataclasses, PipelineContext field changes (review_threads, first_push_done, base_branch; removal of pr_url), base_branch computation in main(), and the parse_json_list_output() helper.

Changes

• plugins/dev-team/scripts/dev_team.py — Added ReviewComment and ReviewThread dataclasses; added review_threads, first_push_done, base_branch fields to PipelineContext; removed pr_url; updated save()/load() for all new/removed fields; added parse_json_list_output(); added base_branch computation block with guard in main()
• plugins/dev-team/scripts/test_dev_team.py — New file; 34 unit tests covering all exit criteria (dataclass fields, serialization round-trips, camelCase JSON wire format, parse_json_list_output() edge cases)
• plugins/dev-team/scripts/validate-tests.cmd — Updated from no-op stub to invoke pytest with PYTHONPATH pointing at the scripts directory
• plugins/dev-team/scripts/validate-tests.sh — Same update for Linux/macOS

Design decisions

• review_threads section is written unconditionally even when the list is empty ([]), to avoid stale values on resume — same rationale applied to review_notes
• base_branch "prefer main on tie" is implemented by initialising best_candidate = "main" and only updating on strict count < best_count, so main wins ties without extra logic
• pr_details removal was a no-op — confirmed absent from the current file (never ported from AdaptiveRemote or removed in ADR-265)
• Removing pr_url intentionally leaves broken ctx.pr_url references in ReviewStep, SignoffStep, and FixPrStep; ADR-254 fixes those step classes
• parse_json_list_output() tries fenced code blocks first (from the end), then bare lines — the reverse of parse_json_output(), per spec intent

[jodavis-claude]

ADR-253 Review

Overall the implementation is clean and matches the spec. One correctness gap in the load() exception handler, noted below.

Exit criteria verified:

• ✅ ReviewComment and ReviewThread dataclasses with correct fields
• ✅ review_threads serialises/deserialises via <!-- section:Review Threads --> as camelCase JSON
• ✅ first_push_done serialises to/from frontmatter as lowercase true/false
• ✅ pr_url removed from PipelineContext; pr_details was already absent (no action needed)
• ✅ base_branch populated in main() with guard (if not ctx.base_branch:), correct tie-break (main wins), and ctx.save() called immediately after
• ✅ parse_json_list_output() tries fenced blocks first, then bare lines, returns [] on failure
• ✅ Tests cover round-trips, camelCase serialisation, and parse_json_list_output edge cases
• ✅ Validate scripts correctly moved to scripts/ (where run_validate_script already looks)

[jodavis-claude]

Sign-off Review — ADR-253

Sign-off result: Approved

Prior review thread

• [P1 — Correctness] Missing TypeError in review_threads exception handler — Resolved. The developer added TypeError to the except tuple on line 294 and added three test cases covering null, dict, and string JSON values. All three cases now produce [] as expected. Thread is resolved on GitHub.

Files scanned (modified since first review)

• plugins/dev-team/scripts/dev_team.py — single-line fix, correct
• plugins/dev-team/scripts/test_dev_team.py — one new test added, all 35 tests pass
• scripts/validate-tests.cmd, scripts/validate-tests.sh — path fix is correct; local copies under plugins/dev-team/scripts/ correctly removed to avoid duplication
• _spec_DevTeamPortability.md — minor wording update only, no logic impact

No new issues found

All exit criteria are met:

• ReviewComment / ReviewThread dataclasses present with correct fields
• review_threads serialises as camelCase JSON under <!-- section:Review Threads --> (unconditionally, even when empty)
• first_push_done serialises as lowercase true/false in frontmatter
• pr_url and pr_details absent from PipelineContext
• base_branch computed in main() with guard and saved before DevTeamPipeline.run()
• parse_json_list_output() exists, fenced-block-first, returns [] on failure
• 35/35 unit tests pass