Skip to content

docs: v1.2 wave-based roadmap + audit reports#107

Merged
TexasCoding merged 2 commits into
mainfrom
docs/v1.2-roadmap-plan
May 17, 2026
Merged

docs: v1.2 wave-based roadmap + audit reports#107
TexasCoding merged 2 commits into
mainfrom
docs/v1.2-roadmap-plan

Conversation

@TexasCoding

@TexasCoding TexasCoding commented May 17, 2026

Copy link
Copy Markdown
Owner

Summary

Lands the planning artifacts before Wave 1 execution starts so the orchestrator works off main.

Test plan

  • Docs-only change, no code touched

Maps the 30 issues from the Wave 5 audit swarm (#77-#106) onto a 5-wave
execution plan mirroring the parallel-agent pattern that produced v1.1.

- Wave 1: 5 parallel low-risk fixes (missing exports, correctness, perf,
  CI pinning, spec-sync hardening)
- Wave 2: 6 parallel test-coverage backfills
- Wave 3: 3 parallel WS overhauls by file boundary (orderbook, dispatcher,
  recv loop)
- Wave 4: backpressure & gap correctness (depends on Wave 3 recv-loop
  changes)
- Wave 5: type cleanup + polish umbrella

Includes:
- Release-cut criteria (which HIGH items must merge before tag).
- Execution conventions carried from v1.1: general-purpose agents only
  (the octo droids hung/hallucinated in Wave 5 audits), explicit CWD
  in every Bash call, isolated worktrees per branch, sequential merges
  between waves.
- Deferred items from v1.1 (#45 prod creds, #53 nested $ref) noted as
  non-blockers for v1.2.

No code changes. Pure planning doc.
@claude

claude Bot commented May 17, 2026

Copy link
Copy Markdown

Code Review — PR #107

Pure documentation PR. Only ROADMAP.md is changed. No code, tests, or config touched. This review focuses on plan correctness and document quality.


Overview

Replaces a 9-line stub with a 105-line wave-based execution plan for 30 audit issues (#77–#106). The structure mirrors the v1.1 wave pattern and encodes hard-won execution conventions from the v1.1 audit swarm. The approach is sound and the dependencies are well-reasoned.


Accuracy check

All 30 issues (#77–#106) are accounted for across the five waves — no gaps, no double-counted issues. File-boundary isolation in Waves 1 and 3 was spot-checked:

  • Wave 1 branches touch disjoint files ✓
  • Wave 3 branches split cleanly across orderbook.py, dispatch.py, and client.py + channels.py
  • docs/RELEASING.md referenced in the release criteria exists ✓

Suggestions

1. Wave 5 dependency ambiguity
The PR summary table marks Wave 5 as "— (can slot in anywhere)" but the document body implies it's last. #90 (type-annotation drift) and #106 (polish backlog) are genuinely independent of Waves 1–4. Making this explicit in the document would give orchestrators permission to start those branches earlier, reducing end-to-end cycle time:

-### Wave 5 — Type-annotation cleanup + polish backlog
+### Wave 5 — Type-annotation cleanup + polish backlog (independent, can start any time)

Or add a one-liner below the table: "These branches have no code dependencies and can be worked concurrently with any other wave."

2. #98 includes a public API change — consider flagging it
The Wave 2 table notes #98 as "(test + public API kwarg)". Silently bundling a public API surface change into a test-coverage wave increases risk. A short note here — or an explicit reminder in the agent prompt — would prevent a reviewer from merging it without API scrutiny.

3. Wave 2 table is missing the Scope column that Wave 1 has
Minor consistency issue. Wave 1's three-column layout (Branch | Issues | Scope) communicates more at a glance. Wave 2 drops Scope entirely. Either add a Scope column to Wave 2 or remove it from Wave 1 for consistency.

4. HIGH severity not marked inline in the wave tables
The release criteria lists #77, #78, #79, #89, #92, #97 as HIGH severity, but this isn't visible in the wave tables themselves. An agent starting Wave 1 won't see that #89 and #92 are HIGH without cross-referencing the release criteria. Consider adding ⚠ HIGH or [HIGH] annotations inline in the branch table rows for those issues.


Minor nits

  • The ## Shipped section at the bottom is cut off in the diff — presumably intentional (pre-existing content untouched). Confirmed this is fine.
  • The deferred v1.1 items (#45, #53) are correctly noted as non-blockers and their preconditions are clearly stated. Good.

Summary

The plan is accurate, the wave sequencing is logically sound, and the execution conventions section captures important institutional knowledge (general-purpose agents only, explicit CWD, worktree isolation). The four suggestions above are improvements, not blockers — this is ready to merge as-is.

🤖 Reviewed with Claude Code

@TexasCoding TexasCoding changed the title docs(roadmap): add v1.2 wave-based implementation plan for audit findings docs: v1.2 wave-based roadmap + audit reports May 17, 2026
@TexasCoding TexasCoding merged commit 3ab4617 into main May 17, 2026
4 checks passed
@TexasCoding TexasCoding deleted the docs/v1.2-roadmap-plan branch May 17, 2026 11:37
@claude

claude Bot commented May 17, 2026

Copy link
Copy Markdown

Code Review — PR #107: docs: v1.2 wave-based roadmap + audit reports

Overview

This is a docs-only PR that lands five post-v1.1.0 audit reports (.planning/audit-{N,O,P,Q,R}-*.md), a v1.1 execution history doc (.planning/v1.1-waves.md), and a substantially updated ROADMAP.md with the v1.2 wave-based plan. No production code is touched. The audit reports are the primary artifact; the roadmap and execution doc follow from them.


Audit Report Quality ✅

The audit format is excellent: every finding has a clear severity tag, a specific file:line pointer, an Evidence: block (often including exact code snippets), and a Suggested fix:. That level of precision makes the findings directly actionable for the implementation agents. The severity mix (HIGH/MEDIUM/LOW/INFORMATIONAL) looks well-calibrated — HIGH findings are genuinely correctness-breaking, not just style issues.


Specific Concerns

1. Inconsistency: v1.1-waves.md says "local-only (not committed to repo)"

.planning/v1.1-waves.md, line 3: This is local-only (not committed to repo).

This comment is now false — the file is being committed in this very PR. Should be removed or updated to reflect that it's been promoted to the repo for historical reference.

2. F-P-02 + F-P-09 are HIGH severity but appear deferred to Wave 3

F-P-02 describes a correctness hole where ERROR-overflow drops do not prevent the orderbook and sequence tracker from advancing — a consumer can have a stale/corrupted local book with no sequence gap detected. F-P-09 describes _handle_seq_gap clearing only tickers[0] when a multi-ticker subscription gaps, silently leaving the other books corrupted.

Both are correctness bugs, not code quality issues. The roadmap groups all WS fixes under #77–#86 but schedules them in Wave 3 (after test-coverage backfill in Wave 2). For a trading SDK, silent orderbook corruption is a higher-priority risk than most test-gap issues. Consider swapping F-P-02 and F-P-09 into Wave 1 or explicitly documenting why Wave 2 ordering was chosen.

3. F-P-01: resubscribe_all is both HIGH severity and a prerequisite for the reconnect tests in Wave 2

The roadmap schedules test coverage for reconnect scenarios (#97, #98) in Wave 2, but the underlying resubscribe_all partial-failure bug (F-P-01, channels.py:199-228) is tracked in Wave 3. This means Wave 2 test agents writing reconnect tests will be writing tests against a broken implementation, which wastes effort or forces them to mark the tests xfail. If F-P-01 can be fixed independently (it touches only channels.py), promoting it to Wave 1 would unblock the Wave 2 test agents.

4. F-O-01 (spec-sync supply chain) is HIGH but the mitigation is nuanced

The finding correctly identifies that spec-sync.yml fetches upstream YAML with contents: write + pull-requests: write and no hash pinning. The three mitigations offered are all valid. The roadmap assigns this to infra/issue-92-spec-sync-hardening in Wave 1, which is the right priority call. One thing worth noting for the implementation agent: the simplest mitigation (require manual PR approval + "I read _generated/models.py" checkbox) is already partially enforced if the repo has branch protection requiring review on PRs from automated workflows — worth checking before adding hash pinning overhead.

5. Wave 2 coverage items for F-O-08 (Retry-After negative/NaN) depend on the Wave 1 fix

#97 (test coverage backfill for retry behavior) is in Wave 2 and explicitly depends on the Retry-After validator fix from #94 (Wave 1). The roadmap calls this out correctly — just confirming the ordering is intentional.

6. F-R-10 documentation gap (RSA signing on every retry blocks async event loop)

Audit R correctly marks this as "document, don't change" and suggests run_in_executor as an opt-in fix. The ROADMAP.md lists this under #103–#106 but the performance issues are bundled without distinguishing which are "fix" vs. "document". For the implementation agents, it would help to note explicitly that F-R-10 is documentation-only so they don't accidentally add run_in_executor complexity.


Roadmap Structure ✅

The five-wave structure is sound and mirrors the v1.1 execution successfully. The disjoint-file constraint per wave reduces merge-conflict risk significantly. A few minor suggestions:

  • Wave 2 lists 6 agents — that's at the edge of manageable parallel coordination. Consider whether any two coverage items share a test file (e.g., test_async_client.py is likely touched by multiple agents covering F-Q-03, F-Q-06, F-Q-12).
  • "Deferred / blocked" section should probably note explicitly that #45 (verify json={} in prod) is NOT part of the v1.2 scope, to avoid confusion with the new WS/security issues that are scope.

Minor Nits

  • AGENTS.md and CLAUDE.md GitNexus index counts bumped (5819→5829, 12421→12434): these look like stale-at-time-of-write values from the audit vs. current. If the GitNexus index was refreshed mid-audit, these numbers may drift again before v1.2 agents start. The auto-update pattern is fine; just noting the fragility.
  • Audit O's clean findings section (F-O-06, F-O-10, F-O-13) is a nice touch — documenting things that were checked and correct saves future auditors from re-auditing them.

Summary

The audit reports are high quality and the roadmap is well-structured. The main actionable feedback:

  1. Fix the self-contradicting comment in v1.1-waves.md about it being "local-only".
  2. Consider promoting F-P-01, F-P-02, F-P-09 (correctness bugs in WS reconnect + orderbook corruption) to Wave 1 — they're independent file changes and should not block behind test coverage work.
  3. Check for shared test-file conflicts among the 6 parallel Wave 2 agents before execution starts.

Otherwise, this is exactly the right way to land planning artifacts before starting parallel execution — the audit evidence is traceable, the issue cross-references are solid, and the wave sequencing is logical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant