docs(v0.3.0): research + 18 plans for fat-milestone planning#87
Merged
Conversation
Phase 5 research: prefilled-URL GitHub Issues pattern, default-deny payload, redaction layer, two-step consent UX, rate/dedup/recursion safeguards, aggregation across Python/Rust/React error producers. Builds on Phase 1's links.py + errorDocsMap deeplink infrastructure; uses already-installed @tauri-apps/plugin-opener (^2.5.4). No new packages required. Covers REPORT-01..12 with confidence levels, 8 pitfalls, subprocess-engine error capture handoff to Phase 2, security domain mapped to ASVS, and 3-wave delivery plan (redactor + payload, consent UI, aggregation + pre-submit search). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
15 new plan files + 2 ADR decision docs across 5 phases. Combined with Phase 1's 3 plans, the v0.3.0 milestone now has 18 PLAN.md files covering all 7 phases (Phase 0 already complete via PR #71). PHASE 2 (Engine Isolation — 4 plans): - 02-01: SubprocessBackend primitive + echo sidecar POC + graceful is_available wrap (ENGINE-01/05) - 02-02: _safe_torchaudio_save helper + migrate 11 WAV write sites + #48 regression (BUG-01) - 02-03: IndexTTS sidecar entry + venv-probe bootstrap + IndexTTS2Backend rewire (ENGINE-02/03/04/07, closes #42) - 02-04: Engine Compatibility Matrix UI + /engines/{id}/health route (ENGINE-06) PHASE 3 (Supertonic-3 + Mirror — 2 plans): - 03-01: Supertonic-3 engine on SubprocessBackend + SHA pin + license gate (TTS-01..06) - 03-02: bootstrap.rs mirror cascade + UV_DEFAULT_INDEX migration + frozen enforcement + docs (INST-07..11) PHASE 4 (Spike-first Adaptive & Specialty — 2 plans + 2 ADRs): - 04-01: OmniVoice-GGUF hardware-adaptive engine + quant_map + bundled binaries (SPIKE-01, GGUF-01..06) - 04-02: OmniVoice-Singing subclass + dub pipeline singing mode + segment detector (SPIKE-02, SING-01..05) - SPIKE-01-gguf.md + SPIKE-02-singing.md ADRs in .planning/decisions/ PHASE 5 (Opt-in Bug Reporting — 3 plans): - 05-01: Redactor + BugReporter + URL builder + rate/dedup/recursion safeguards + FastAPI router (REPORT-01/02/03/05/06/07/08/10/11) - 05-02: BugReportDialog two-step consent + PrivacyPanel + ErrorBoundary integration + Rust panic hook (chained) (REPORT-01-Rust/04/09/12) - 05-03: Dry-run vs 3 historical issues + cross-platform openUrl smoke + Phase 2 subprocess-errors handoff (REPORT-02 smoke, REPORT-03 expansion, REPORT-09) PHASE 6 (Release + Retro — 4 plans): - 06-01: rc1 prep — version bump across 4 sources + CHANGELOG + retro stub + PR-73-strategy doc (REL-01/03/06) - 06-02: CI guards — workflow-parity actionlint + tag-shaped dry-run (Phase 0 retro options B + C; closes release-engineer gap) - 06-03: PR #73 reimplementation (NOT rebase) — backend-split installer with mirror-cascade integration + pill-mode regression checkpoint - 06-04: Execute the release — pre-tag gates + 4-OS clean-VM + 48h soak + tag + retro + 3 v0.4 deferral tracking issues (REL-01/02/03/04/05/06) Scope decisions locked in plans (council session): - SoniTranslate refactor DEFERRED to v0.4 (Phase 2 ships SubprocessBackend without migrating Soni) - macOS notarization DEFERRED to v0.4 (Phase 6 ships xattr -cr automation per CLAUDE.md Key Decision #7) - supertonic pin 1.2.3 → 1.3.1 (already committed in ba63733) - SPIKE-01 and SPIKE-02 both GO; 13/13 Phase 4 reqs stay in scope - PR #73 reimplemented, not rebased (93 commits behind main) All 18 plans validated via gsd-sdk frontmatter.validate + verify.plan-structure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR adds comprehensive planning and research documentation for OmniVoice-Studio's v0.3.0 release across Phases 2–6, including infrastructure work, new engine integrations, bug reporting, and release procedures, with minor Supertonic version metadata updates. Changesv0.3.0 Release Planning & Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes
✨ Finishing Touches🧪 Generate unit tests (beta)
|
4 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (17)
.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-01-PLAN.md-452-460 (1)
452-460:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTerminate the STRIDE table cleanly before the closing tag.
The table is being linted as malformed at the closing line. Add a blank line between the final table row and
</threat_model>so markdownlint stops treating the tag as a table row.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-01-PLAN.md around lines 452 - 460, The STRIDE table ends immediately before the closing tag which causes markdownlint to parse the closing tag as a table row; insert a single blank line between the final table row (the row containing "T-03-SC | Tampering | supertonic supply chain") and the closing </threat_model> tag so the table is terminated cleanly and linting passes..planning/phases/04-adaptive-specialty-engines-spike-first/04-02-PLAN.md-281-289 (1)
281-289:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClose the STRIDE table cleanly before
</threat_model>.Add a blank line after the last row so the closing tag is not parsed as a malformed table row.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-02-PLAN.md around lines 281 - 289, The STRIDE markdown table isn't terminated cleanly which makes the closing tag </threat_model> parse as a table row; edit the table block (the final rows including the T-04-12 row and the table footer) to add a blank line after the last markdown table row and ensure the table delimiter/pipe lines end before the </threat_model> tag so the table is closed properly before the tag..planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-02-PLAN.md-325-332 (1)
325-332:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix malformed table termination in STRIDE section.
The table appears to bleed into the closing tag and triggers MD055/MD056. Insert a blank line after the final table row before
</threat_model>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-02-PLAN.md around lines 325 - 332, The markdown table in the STRIDE section is not properly terminated which causes MD055/MD056; fix this by inserting a single blank line after the last table row (after the T-03-10 row) and before the closing </threat_model> tag so the table is separated from the XML/HTML close tag; ensure the last row (T-03-10) remains unchanged and only a blank line is added..planning/phases/04-adaptive-specialty-engines-spike-first/04-02-PLAN.md-164-164 (1)
164-164:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSwitch indented snippets to fenced code blocks (MD046).
Converting these to fenced blocks will resolve lint warnings and keep markdown rendering stable across viewers.
Also applies to: 206-206
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-02-PLAN.md at line 164, The markdown contains indented code snippets (e.g., the snippet following the line "Read `backend/services/dub_pipeline.py` and identify:" and the similar occurrence later) which trigger MD046; replace each indented snippet with fenced code blocks using triple backticks (```), add an appropriate language tag if known (e.g., ```python), and ensure opening and closing backticks wrap the code exactly to preserve formatting and fix the lint warnings..planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-01-PLAN.md-243-243 (1)
243-243:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse fenced blocks instead of indented code blocks to satisfy markdownlint MD046.
These sections are being parsed as indented code blocks. Converting them to fenced blocks will remove lint warnings and keep rendering consistent.
Suggested pattern
- 1. Extend `backend/services/settings_store.py` with two helpers: - def get_license_accepted(engine_id: str) -> bool: - ... + 1. Extend `backend/services/settings_store.py` with two helpers: + + ```python + def get_license_accepted(engine_id: str) -> bool: + ... + ```Also applies to: 288-288, 341-341, 408-408
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-01-PLAN.md at line 243, Indented code blocks in the document are being parsed as indented code and trigger markdownlint MD046; replace each indented example block with fenced code blocks using triple backticks and an appropriate language tag (e.g., ```python) and a closing ``` so examples like the snippet containing the function signature def get_license_accepted(engine_id: str) -> bool: are wrapped as ```python ... ```; apply the same fenced-block conversion to the other similar sections mentioned (the other occurrences of indented examples)..planning/phases/04-adaptive-specialty-engines-spike-first/04-RESEARCH.md-117-117 (1)
117-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks (MD040).
Please annotate these fences (e.g.,
textfor architecture diagrams) so markdownlint passes consistently.Suggested edit style
-``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ ... -``` +```Also applies to: 165-165, 207-207
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-RESEARCH.md at line 117, Add explicit language identifiers to the fenced code blocks that contain architecture diagrams and plain text so markdownlint rule MD040 is satisfied; locate the three fenced blocks that currently use bare triple backticks (the ASCII diagram and the other two blocks mentioned) and change their opening fences from ``` to ```text (or another appropriate language like ```dot for diagrams) so each code fence is annotated consistently..planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md-191-191 (1)
191-191:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize indented code samples to fenced blocks (MD046).
These list-embedded snippets should be fenced to avoid markdownlint warnings and improve renderer consistency.
Also applies to: 225-225, 263-263, 270-270
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md at line 191, Normalize the indented/list-embedded code examples into fenced code blocks: locate the list item that currently shows Create `backend/engines/omnivoice_gguf/__init__.py` (and the three other list-embedded snippets flagged in this document) and replace the indented inline snippet style with proper triple-backtick fenced code blocks (specify language where appropriate), ensuring opening and closing fences surround the code so markdownlint MD046 is satisfied..planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md-306-315 (1)
306-315:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a separator line between the STRIDE table and closing tag.
The current layout causes the closing tag to be interpreted as a malformed table row.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md around lines 306 - 315, The STRIDE table currently runs directly into the XML closing tag and is being parsed as a table row; insert a separator line between the table and the closing tag by adding a blank line or a Markdown horizontal rule (e.g., a line with three dashes '---') immediately before </threat_model> in .planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md so the closing tag is not treated as table content..planning/phases/05-opt-in-bug-reporting/05-01-PLAN.md-178-178 (1)
178-178:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify a language on the interface fenced block.
Line 178 has an unlabeled code fence (MD040). Add a language (e.g.,
text) for lint compliance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/05-opt-in-bug-reporting/05-01-PLAN.md at line 178, The markdown contains an unlabeled fenced code block marked only by ``` which triggers MD040; update that fence to include a language identifier (for example change the opening ``` to ```text) so the block is language-labeled and lint-compliant, leaving the fence contents unchanged..planning/phases/06-release-verification-retro/06-02-PLAN.md-74-74 (1)
74-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifier to the parity-check fenced block.
Line 74 uses an unlabeled fenced block; add
bashto satisfy MD040 and keep workflow docs lint-clean.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/06-release-verification-retro/06-02-PLAN.md at line 74, The unlabeled fenced code block (the triple backtick parity-check block) should be updated to include the language identifier by replacing the opening ``` with ```bash so the block is labeled as bash (this satisfies MD040 and keeps the workflow docs lint-clean)..planning/phases/05-opt-in-bug-reporting/05-02-PLAN.md-168-168 (1)
168-168:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the interface fenced block.
Line 168 uses an unlabeled fence, triggering MD040. Use
text/jsonas appropriate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/05-opt-in-bug-reporting/05-02-PLAN.md at line 168, The fenced code block that contains the interface example is unlabeled and triggers MD040; update its opening fence from ``` to a language tag such as ```text (or ```json if the block is valid JSON) so the block is explicitly labeled and the linter warning is resolved..planning/phases/06-release-verification-retro/06-01-PLAN.md-32-32 (1)
32-32:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix milestone count wording to match the documented issue set.
Line 32 says “all 11 originally-open + addition issues,” but the plan enumerates 13 issue IDs later. Align this count text to avoid release gating confusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/06-release-verification-retro/06-01-PLAN.md at line 32, Update the milestone count wording on the string "GitHub Milestone `v0.3.0` exists and contains all 11 originally-open + addition issues" to match the enumerated issue set (13 IDs) later in the plan; replace the quoted phrase with wording such as "GitHub Milestone `v0.3.0` exists and contains all 13 originally-open + additional issues" or "GitHub Milestone `v0.3.0` exists and contains the 13 issue IDs listed below" so the text in the line highlighted (the quoted milestone sentence) aligns with the documented issue list..planning/phases/06-release-verification-retro/06-RESEARCH.md-162-162 (1)
162-162:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLabel diagram/structure fenced blocks with a language.
Line 162 and Line 211 are unlabeled fences (MD040). Add
textfor both.Also applies to: 211-211
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/06-release-verification-retro/06-RESEARCH.md at line 162, There are two unlabeled fenced code blocks represented by triple backticks (```) in the document; update each of those fences to include the language label "text" (i.e., change ``` to ```text) so the Markdown linter rule MD040 is satisfied for both occurrences..planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md-181-181 (1)
181-181:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced blocks to satisfy markdownlint.
Line 181 and Line 237 use unlabeled fenced blocks, which keeps triggering MD040 and can break docs lint gates.
Suggested edit
-``` +```text ┌─────────────────────┐ ┌─────────────────────┐ ┌────────────────────────┐ │ Python error │ │ Rust panic │ │ React error / console │ ... -``` +``` -``` +```text backend/ ├── services/ │ ├── bug_report.py ... -``` +```Also applies to: 237-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md at line 181, Two unlabeled fenced code blocks in .planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md (the ASCII diagram block around the Python/Rust/React diagram and the backend tree block) trigger markdownlint MD040; fix by adding a language identifier (use "text") after the opening triple backticks for both fenced blocks so they read ```text. Locate the unlabeled blocks by matching their contents (the ASCII diagram and the "backend/ ├── services/ │ ├── bug_report.py" tree) and update the opening fence for each to include the "text" label..planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md-497-497 (1)
497-497:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify URL-cap claim wording to avoid over-precision.
Line 497 states a concrete reliability cutoff (“~8 KB falls off; ~6 KB safe”) while the same doc marks this as community consensus, not an official limit. Tighten wording to “conservative cap” to avoid treating it as a hard platform guarantee.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md at line 497, Update the sentence in .planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md that currently reads “~8 KB encoded is where reliability falls off; ~6 KB is the safe upper bound” to soften the claim — replace it with wording that frames this as a community-recommended conservative cap (e.g., “community-recommended conservative cap of ~6 KB encoded, as reliability appears to degrade around ~8 KB”) so the doc avoids implying an official GitHub platform limit; keep the existing mention of community consensus (sindresorhus/new-github-issue-url and GitHub Community threads) for attribution..planning/phases/06-release-verification-retro/06-RESEARCH.md-775-775 (1)
775-775:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape
||in the table cell to fix broken column parsing.Line 775 includes
inputs.draft || 'true'inside a markdown table row, which is parsed as extra columns (MD056). Escape the pipes or rewrite the expression.Suggested edit
-| A6 | release.yml's `draft: ${{ inputs.draft || 'true' }}` default applies to tag pushes too (publishes as draft) | Anti-Patterns: "Forgetting `gh release edit --draft=false`" | Need to verify by reading tauri-action's handling of `releaseDraft`; if tag-push actually publishes non-draft by default, the "promote step" in the runbook isn't needed. | +| A6 | release.yml's `draft: ${{ inputs.draft \|\| 'true' }}` default applies to tag pushes too (publishes as draft) | Anti-Patterns: "Forgetting `gh release edit --draft=false`" | Need to verify by reading tauri-action's handling of `releaseDraft`; if tag-push actually publishes non-draft by default, the "promote step" in the runbook isn't needed. |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/06-release-verification-retro/06-RESEARCH.md at line 775, The markdown table row contains the expression release.yml's draft: ${{ inputs.draft || 'true' }} which breaks column parsing (MD056); fix it by escaping the pipe characters in the `||` operator or by wrapping the entire expression in inline code so the table cell is treated as one column; update the cell that currently contains "release.yml's `draft: ${{ inputs.draft || 'true' }}`" to use an escaped form (e.g., backslash-escaped pipes) or surround the expression with backticks to prevent the table from splitting into extra columns..planning/phases/05-opt-in-bug-reporting/05-01-PLAN.md-209-209 (1)
209-209:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEscape or code-wrap
redact(redact(x)[0])to avoid MD011 parse errors.Line 209 is parsed as reversed link syntax because of
(x)[0]. Wrap the expression in backticks or escape brackets to keep markdown parsing stable.Suggested edit
- - **Idempotency:** `redact(redact(x)[0])[0] == redact(x)[0]` for any x — explicit unit test + - **Idempotency:** `redact(redact(x)[0])[0] == redact(x)[0]` for any x — explicit unit test🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/05-opt-in-bug-reporting/05-01-PLAN.md at line 209, The markdown line containing the idempotency property uses raw bracket syntax which triggers MD011; wrap the whole expression redact(redact(x)[0])[0] == redact(x)[0] in inline code ticks (e.g., `redact(redact(x)[0])[0] == redact(x)[0]`) or escape the bracket characters (e.g., redact(redact(x)\[0\])\[0\] == redact(x)\[0\]) so the parser treats it as code rather than link syntax; update the line that references the redact function and the specific expression accordingly.
🧹 Nitpick comments (2)
.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-04-PLAN.md (1)
205-220: ⚡ Quick winUse a family-scoped health route to avoid future ID collisions.
/engines/{engine_id}/healthrequires cross-registry scanning and becomes ambiguous if TTS/ASR/LLM ever share IDs. Prefer/engines/{family}/{engine_id}/healthnow to keep contract deterministic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-04-PLAN.md around lines 205 - 220, The current engine_health handler and route use a single path GET /engines/{engine_id}/health and scan all registries (tts_backend._REGISTRY, asr_backend._REGISTRY, llm_backend._REGISTRY), which is ambiguous; change the route to GET /engines/{family}/{engine_id}/health and update the engine_health function signature to accept family: str, validate family ∈ {"tts","asr","llm"}, map family to the correct registry (e.g., {"tts": tts_backend._REGISTRY, "asr": asr_backend._REGISTRY, "llm": llm_backend._REGISTRY}), then look up engine_id only in that registry and raise HTTPException(404) for unknown family or missing engine_id..planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-01-PLAN.md (1)
288-288: ⚡ Quick winAvoid
_work_queue.qsize()for GPU-slot assertions.That relies on
ThreadPoolExecutorinternals and is brittle across Python/platform variants. Prefer asserting observable behavior (e.g., subsequent acquire/generate succeeds within timeout) instead of private queue state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-01-PLAN.md at line 288, The test test_sidecar_dies_releases_gpu_slot should stop inspecting ThreadPoolExecutor internals (avoid _get_gpu_pool()._work_queue.qsize()) and instead assert observable behavior: after launching the sidecar with OMNIVOICE_ECHO_CRASH=1 and the sidecar exits, attempt to acquire a GPU slot or run a short generate/openai-like request using the same GPU pool (via _get_gpu_pool() acquire/submit or the public generate API used by other tests) and assert it completes within a reasonable timeout; update the test to wait for the sidecar exit, then perform the acquire/generate and fail the test if it times out, ensuring you reference test_sidecar_dies_releases_gpu_slot and _get_gpu_pool() in the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.planning/decisions/SPIKE-02-singing.md:
- Around line 35-40: The current rule in OmniVoiceSingingBackend.generate() only
skips prepending when the prompt.startsWith('['), which lets prompts like
'[happy] ...' bypass the required singing tag and cause garbled output; change
the logic to check for the presence of the singing tag (e.g.,
prompt.includes('[singing]') with case-insensitive match) and only skip
prepending if that tag exists, otherwise always prepend '[singing]' to the
prompt before synthesis so power-user bracketed modifiers still work.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-01-PLAN.md:
- Line 283: The sidecar_script() function currently builds its path using
Path(__file__).parent.parent.parent which resolves relative to the tests tree
(tests/backend/...), causing wrong targets; update sidecar_script() to compute a
repo-root-anchored path instead (e.g., resolve Path(__file__).resolve() and walk
parents until you find a repo marker like pyproject.toml or the top-level
"backend" directory, or use git to locate repo root) so the returned path points
to the repo-root backend/engines/_echo/main.py and fixes failures seen in
tests/backend/services/test_subprocess_backend.py.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-02-PLAN.md:
- Line 276: The test's Phase 0 fixture probe currently marks the missing fixture
as xfail which contradicts the requirement to fail CI; change the probe so that
when tests/fixtures/sample_5s.mp4 is not present it calls pytest.fail (with a
clear message like "Phase 0 fixture missing — add tests/fixtures/sample_5s.mp4
before running.") instead of using xfail, updating the test/fixture check logic
(the "Phase 0 fixture probe") to assert failure via pytest.fail.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-03-PLAN.md:
- Line 256: The stub indextts package created under tmpdir/site-packages isn't
visible to the subprocess used by the _venv_can_import_indextts probe; update
tests/backend/services/test_indextts_backward_compat.py to make the stub
importable by the probed interpreter either by (A) setting the subprocess
environment PYTHONPATH to include tmp_path/"site-packages" before running python
-c "import indextts.infer_v2" or (B) resolving the target venv's real
site-packages path (via sysconfig or the venv's python -c query) and writing the
stub files into that site-packages location; use the existing
monkeypatch/tmp_path helpers to implement the chosen approach so the subprocess
import succeeds.
In
@.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-RESEARCH.md:
- Around line 324-325: The import for SubprocessBackend is using the
non-canonical module path (engines._subprocess); change it to import the
primitive from the canonical Phase 2 location by importing SubprocessBackend
from backend.services.subprocess_backend so all references use the same module
(SubprocessBackend) and avoid implementation drift.
- Line 35: Pick a single mirror-rotation policy and make the document
consistent: either (A) allow runtime rotation of the allow-list (mirrors.json in
frontend/src-tauri/resources/mirrors.json) without a release, or (B) require app
updates to rotate mirrors. Locate and update the two contradictory statements
(the sentence asserting "rotatable without release" and the sentence stating
"rotation happens only via app updates") so they both state the chosen policy,
and update any dependent wording (e.g., architecture/requirements references) to
match.
In @.planning/phases/06-release-verification-retro/06-04-PLAN.md:
- Line 183: Several <verify><automated> XML blocks contain HTML-escaped shell
operators (&& and &) which will break when executed; open each
<verify><automated> block (the ones used for Task 1, Task 4, Task 5, Task 7 and
the final verification) and replace the escaped sequences && and &
with the actual shell operators && and & in the GH CLI commands (e.g., the gh
release view ... and gh run list ... command lines) so the commands run
correctly in a shell.
- Line 181: The stderr redirection in the `<action>` command contains an
XML-escaped ampersand (`2>&1`) which breaks the shell redirection; update
the action text that runs `gh release view v0.3.0-rc1 2>&1 | grep -q
"release not found"` to use the correct shell token `2>&1` (i.e., replace
`2>&1` with `2>&1`) so the pipeline `gh release view v0.3.0-rc1 2>&1 | grep
-q "release not found"` works as intended.
---
Minor comments:
In
@.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-01-PLAN.md:
- Around line 452-460: The STRIDE table ends immediately before the closing tag
which causes markdownlint to parse the closing tag as a table row; insert a
single blank line between the final table row (the row containing "T-03-SC |
Tampering | supertonic supply chain") and the closing </threat_model> tag so the
table is terminated cleanly and linting passes.
- Line 243: Indented code blocks in the document are being parsed as indented
code and trigger markdownlint MD046; replace each indented example block with
fenced code blocks using triple backticks and an appropriate language tag (e.g.,
```python) and a closing ``` so examples like the snippet containing the
function signature def get_license_accepted(engine_id: str) -> bool: are wrapped
as ```python ... ```; apply the same fenced-block conversion to the other
similar sections mentioned (the other occurrences of indented examples).
In
@.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-02-PLAN.md:
- Around line 325-332: The markdown table in the STRIDE section is not properly
terminated which causes MD055/MD056; fix this by inserting a single blank line
after the last table row (after the T-03-10 row) and before the closing
</threat_model> tag so the table is separated from the XML/HTML close tag;
ensure the last row (T-03-10) remains unchanged and only a blank line is added.
In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md:
- Line 191: Normalize the indented/list-embedded code examples into fenced code
blocks: locate the list item that currently shows Create
`backend/engines/omnivoice_gguf/__init__.py` (and the three other list-embedded
snippets flagged in this document) and replace the indented inline snippet style
with proper triple-backtick fenced code blocks (specify language where
appropriate), ensuring opening and closing fences surround the code so
markdownlint MD046 is satisfied.
- Around line 306-315: The STRIDE table currently runs directly into the XML
closing tag and is being parsed as a table row; insert a separator line between
the table and the closing tag by adding a blank line or a Markdown horizontal
rule (e.g., a line with three dashes '---') immediately before </threat_model>
in .planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md so
the closing tag is not treated as table content.
In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-02-PLAN.md:
- Around line 281-289: The STRIDE markdown table isn't terminated cleanly which
makes the closing tag </threat_model> parse as a table row; edit the table block
(the final rows including the T-04-12 row and the table footer) to add a blank
line after the last markdown table row and ensure the table delimiter/pipe lines
end before the </threat_model> tag so the table is closed properly before the
tag.
- Line 164: The markdown contains indented code snippets (e.g., the snippet
following the line "Read `backend/services/dub_pipeline.py` and identify:" and
the similar occurrence later) which trigger MD046; replace each indented snippet
with fenced code blocks using triple backticks (```), add an appropriate
language tag if known (e.g., ```python), and ensure opening and closing
backticks wrap the code exactly to preserve formatting and fix the lint
warnings.
In @.planning/phases/04-adaptive-specialty-engines-spike-first/04-RESEARCH.md:
- Line 117: Add explicit language identifiers to the fenced code blocks that
contain architecture diagrams and plain text so markdownlint rule MD040 is
satisfied; locate the three fenced blocks that currently use bare triple
backticks (the ASCII diagram and the other two blocks mentioned) and change
their opening fences from ``` to ```text (or another appropriate language like
```dot for diagrams) so each code fence is annotated consistently.
In @.planning/phases/05-opt-in-bug-reporting/05-01-PLAN.md:
- Line 178: The markdown contains an unlabeled fenced code block marked only by
``` which triggers MD040; update that fence to include a language identifier
(for example change the opening ``` to ```text) so the block is language-labeled
and lint-compliant, leaving the fence contents unchanged.
- Line 209: The markdown line containing the idempotency property uses raw
bracket syntax which triggers MD011; wrap the whole expression
redact(redact(x)[0])[0] == redact(x)[0] in inline code ticks (e.g.,
`redact(redact(x)[0])[0] == redact(x)[0]`) or escape the bracket characters
(e.g., redact(redact(x)\[0\])\[0\] == redact(x)\[0\]) so the parser treats it as
code rather than link syntax; update the line that references the redact
function and the specific expression accordingly.
In @.planning/phases/05-opt-in-bug-reporting/05-02-PLAN.md:
- Line 168: The fenced code block that contains the interface example is
unlabeled and triggers MD040; update its opening fence from ``` to a language
tag such as ```text (or ```json if the block is valid JSON) so the block is
explicitly labeled and the linter warning is resolved.
In @.planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md:
- Line 181: Two unlabeled fenced code blocks in
.planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md (the ASCII diagram block
around the Python/Rust/React diagram and the backend tree block) trigger
markdownlint MD040; fix by adding a language identifier (use "text") after the
opening triple backticks for both fenced blocks so they read ```text. Locate the
unlabeled blocks by matching their contents (the ASCII diagram and the "backend/
├── services/ │ ├── bug_report.py" tree) and update the opening fence for each
to include the "text" label.
- Line 497: Update the sentence in
.planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md that currently reads “~8
KB encoded is where reliability falls off; ~6 KB is the safe upper bound” to
soften the claim — replace it with wording that frames this as a
community-recommended conservative cap (e.g., “community-recommended
conservative cap of ~6 KB encoded, as reliability appears to degrade around ~8
KB”) so the doc avoids implying an official GitHub platform limit; keep the
existing mention of community consensus (sindresorhus/new-github-issue-url and
GitHub Community threads) for attribution.
In @.planning/phases/06-release-verification-retro/06-01-PLAN.md:
- Line 32: Update the milestone count wording on the string "GitHub Milestone
`v0.3.0` exists and contains all 11 originally-open + addition issues" to match
the enumerated issue set (13 IDs) later in the plan; replace the quoted phrase
with wording such as "GitHub Milestone `v0.3.0` exists and contains all 13
originally-open + additional issues" or "GitHub Milestone `v0.3.0` exists and
contains the 13 issue IDs listed below" so the text in the line highlighted (the
quoted milestone sentence) aligns with the documented issue list.
In @.planning/phases/06-release-verification-retro/06-02-PLAN.md:
- Line 74: The unlabeled fenced code block (the triple backtick parity-check
block) should be updated to include the language identifier by replacing the
opening ``` with ```bash so the block is labeled as bash (this satisfies MD040
and keeps the workflow docs lint-clean).
In @.planning/phases/06-release-verification-retro/06-RESEARCH.md:
- Line 162: There are two unlabeled fenced code blocks represented by triple
backticks (```) in the document; update each of those fences to include the
language label "text" (i.e., change ``` to ```text) so the Markdown linter rule
MD040 is satisfied for both occurrences.
- Line 775: The markdown table row contains the expression release.yml's draft:
${{ inputs.draft || 'true' }} which breaks column parsing (MD056); fix it by
escaping the pipe characters in the `||` operator or by wrapping the entire
expression in inline code so the table cell is treated as one column; update the
cell that currently contains "release.yml's `draft: ${{ inputs.draft || 'true'
}}`" to use an escaped form (e.g., backslash-escaped pipes) or surround the
expression with backticks to prevent the table from splitting into extra
columns.
---
Nitpick comments:
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-01-PLAN.md:
- Line 288: The test test_sidecar_dies_releases_gpu_slot should stop inspecting
ThreadPoolExecutor internals (avoid _get_gpu_pool()._work_queue.qsize()) and
instead assert observable behavior: after launching the sidecar with
OMNIVOICE_ECHO_CRASH=1 and the sidecar exits, attempt to acquire a GPU slot or
run a short generate/openai-like request using the same GPU pool (via
_get_gpu_pool() acquire/submit or the public generate API used by other tests)
and assert it completes within a reasonable timeout; update the test to wait for
the sidecar exit, then perform the acquire/generate and fail the test if it
times out, ensuring you reference test_sidecar_dies_releases_gpu_slot and
_get_gpu_pool() in the change.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-04-PLAN.md:
- Around line 205-220: The current engine_health handler and route use a single
path GET /engines/{engine_id}/health and scan all registries
(tts_backend._REGISTRY, asr_backend._REGISTRY, llm_backend._REGISTRY), which is
ambiguous; change the route to GET /engines/{family}/{engine_id}/health and
update the engine_health function signature to accept family: str, validate
family ∈ {"tts","asr","llm"}, map family to the correct registry (e.g., {"tts":
tts_backend._REGISTRY, "asr": asr_backend._REGISTRY, "llm":
llm_backend._REGISTRY}), then look up engine_id only in that registry and raise
HTTPException(404) for unknown family or missing engine_id.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64dff68d-de0d-4695-863b-957e10528a71
📒 Files selected for processing (23)
.planning/decisions/SPIKE-01-gguf.md.planning/decisions/SPIKE-02-singing.md.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-01-PLAN.md.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-02-PLAN.md.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-03-PLAN.md.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-04-PLAN.md.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-RESEARCH.md.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-01-PLAN.md.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-02-PLAN.md.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-RESEARCH.md.planning/phases/04-adaptive-specialty-engines-spike-first/04-01-PLAN.md.planning/phases/04-adaptive-specialty-engines-spike-first/04-02-PLAN.md.planning/phases/04-adaptive-specialty-engines-spike-first/04-RESEARCH.md.planning/phases/05-opt-in-bug-reporting/05-01-PLAN.md.planning/phases/05-opt-in-bug-reporting/05-02-PLAN.md.planning/phases/05-opt-in-bug-reporting/05-03-PLAN.md.planning/phases/05-opt-in-bug-reporting/05-RESEARCH.md.planning/phases/06-release-verification-retro/06-01-PLAN.md.planning/phases/06-release-verification-retro/06-02-PLAN.md.planning/phases/06-release-verification-retro/06-03-PLAN.md.planning/phases/06-release-verification-retro/06-04-PLAN.md.planning/phases/06-release-verification-retro/06-RESEARCH.mdCLAUDE.md
Comment on lines
+35
to
+40
| - `omnivoice-singing` returns garbled output if the `[singing]` tag is missing — automatic injection is load-bearing. | ||
|
|
||
| **Mitigations:** | ||
| - Per-segment override available in the dubbing UI before any segment is committed to a render (user owns the final route — SING-03 already requires this). | ||
| - SING-05 acceptance scoped to native-language singing pass; cross-language flagged as best-effort with model-card disclaimer surfaced in the engine card UI. | ||
| - `OmniVoiceSingingBackend.generate()` always prepends `[singing]` unless the prompt already starts with `[`, allowing power users to compose `[singing] [happy]` etc. manually. |
Contributor
There was a problem hiding this comment.
Always enforce [singing] presence, not just “starts with [”.
Line 35 states missing [singing] yields garbled output, but Line 40’s rule allows prompts like [happy] ... to bypass injection and break synthesis. Gate on “contains [singing]” (or prepend it) rather than “starts with [”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.planning/decisions/SPIKE-02-singing.md around lines 35 - 40, The current
rule in OmniVoiceSingingBackend.generate() only skips prepending when the
prompt.startsWith('['), which lets prompts like '[happy] ...' bypass the
required singing tag and cause garbled output; change the logic to check for the
presence of the singing tag (e.g., prompt.includes('[singing]') with
case-insensitive match) and only skip prepending if that tag exists, otherwise
always prepend '[singing]' to the prompt before synthesis so power-user
bracketed modifiers still work.
| Step 3 — Define an `_EchoBackend(SubprocessBackend)` subclass IN THE TEST FILE (not in production code, to keep _REGISTRY clean): | ||
| - `id = "_echo"`, `display_name = "Echo (test)"`. | ||
| - `venv_python()` returns `Path(sys.executable)` — echo sidecar runs under the parent's Python so the test does not require a separate venv. | ||
| - `sidecar_script()` returns `Path(__file__).parent.parent.parent / "backend" / "engines" / "_echo" / "main.py"`. |
Contributor
There was a problem hiding this comment.
sidecar_script() path example resolves to the wrong tree.
From tests/backend/services/test_subprocess_backend.py, Path(__file__).parent.parent.parent / "backend" / ... points under tests/backend/..., not repo-root backend/.... Use a root-anchored resolver (or fd-located absolute path) to avoid false test failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-01-PLAN.md
at line 283, The sidecar_script() function currently builds its path using
Path(__file__).parent.parent.parent which resolves relative to the tests tree
(tests/backend/...), causing wrong targets; update sidecar_script() to compute a
repo-root-anchored path instead (e.g., resolve Path(__file__).resolve() and walk
parents until you find a repo marker like pyproject.toml or the top-level
"backend" directory, or use git to locate repo root) so the returned path points
to the repo-root backend/engines/_echo/main.py and fixes failures seen in
tests/backend/services/test_subprocess_backend.py.
|
|
||
| Step 7 — Write `tests/backend/test_dub_pipeline_wav.py`: | ||
| - Use `from fastapi.testclient import TestClient` + the app factory from `backend/main.py`. | ||
| - Phase 0 fixture probe: check if `tests/fixtures/sample_5s.mp4` exists. If not, the test xfails with a clear message: "Phase 0 fixture missing — add tests/fixtures/sample_5s.mp4 before running." (Do NOT skip silently — the test needs to surface the missing fixture as a CI failure so it gets fixed.) |
Contributor
There was a problem hiding this comment.
xfail conflicts with the stated “must fail CI” requirement.
Line 276 requests xfail for missing fixture but also says absence must be surfaced as a CI failure. Use an explicit assertion/fail (pytest.fail) for missing required fixtures so the gate is enforced.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-02-PLAN.md
at line 276, The test's Phase 0 fixture probe currently marks the missing
fixture as xfail which contradicts the requirement to fail CI; change the probe
so that when tests/fixtures/sample_5s.mp4 is not present it calls pytest.fail
(with a clear message like "Phase 0 fixture missing — add
tests/fixtures/sample_5s.mp4 before running.") instead of using xfail, updating
the test/fixture check logic (the "Phase 0 fixture probe") to assert failure via
pytest.fail.
| - Returns True if either Probe 1 OR Probe 2 path has a Python executable present (file existence only — no import probe; the import probe runs at resolve time, not at every is_available() call which fires on every Settings page render). | ||
| - Cache the resolved path in a module-level variable; clear with `invalidate()` for tests. | ||
|
|
||
| Step 3 — Write `tests/backend/services/test_indextts_backward_compat.py` per `<behavior>`. Use `pytest`'s `monkeypatch` and `tmp_path` heavily. For the `_venv_can_import_indextts` probe, in tests, the stub indextts package can be as small as `mkdir -p {tmpdir}/site-packages/indextts && touch {tmpdir}/site-packages/indextts/__init__.py && touch {tmpdir}/site-packages/indextts/infer_v2.py` — the import only needs to succeed at `import indextts.infer_v2`, the test doesn't care about the contents. |
Contributor
There was a problem hiding this comment.
Stub-package setup is incomplete for the subprocess import probe.
Creating tmpdir/site-packages/indextts/... alone won’t satisfy python -c "import indextts.infer_v2" unless PYTHONPATH/venv site-packages are wired. Add explicit path injection or build the stub in the probed interpreter’s real site-packages.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.planning/phases/02-engine-isolation-subprocessbackend-indextts-wav-export-dubbi/02-03-PLAN.md
at line 256, The stub indextts package created under tmpdir/site-packages isn't
visible to the subprocess used by the _venv_can_import_indextts probe; update
tests/backend/services/test_indextts_backward_compat.py to make the stub
importable by the probed interpreter either by (A) setting the subprocess
environment PYTHONPATH to include tmp_path/"site-packages" before running python
-c "import indextts.infer_v2" or (B) resolving the target venv's real
site-packages path (via sysconfig or the venv's python -c query) and writing the
stub files into that site-packages location; use the existing
monkeypatch/tmp_path helpers to implement the chosen approach so the subprocess
import succeeds.
| 2. **Model revision pinned by commit SHA, not tag** (TTS-03). The model card is being actively updated (5 commits in the last 12 days as of 2026-05-18). A tag pin is not strong enough — pin to a specific SHA. See Open Questions Q2. | ||
| 3. **License acceptance gates first use** (TTS-05). The Supertonic-3 model ships under **OpenRAIL-M** (model weights, restricted-use clauses) while the SDK code is **MIT**. We surface both in the engine card with click-through links and require explicit acceptance before download/first generate. | ||
| 4. **CPU-only honesty** (TTS-04). The engine MUST report CPU-only when CUDA is absent. Because Supertonic-3 has no CUDA path at all in the current SDK, `is_available()` reports `"cpu"` everywhere — no `"mps"` or `"cuda"` paths claimed. | ||
| 5. **Mirror list is allow-list, JSON-shipped** (INST-08, INST-09). User cannot enter freeform URLs (supply-chain risk per Out of Scope table in REQUIREMENTS.md). The allow-list lives in `frontend/src-tauri/resources/mirrors.json` (or equivalent — see Architecture below) and is rotatable without a release. |
Contributor
There was a problem hiding this comment.
Mirror-rotation policy is self-contradictory.
Line 35 says the mirror list is rotatable without release, while Line 658/660 says rotation happens only via app updates. Pick one policy and align all sections to avoid execution-time confusion.
Also applies to: 658-660
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-RESEARCH.md
at line 35, Pick a single mirror-rotation policy and make the document
consistent: either (A) allow runtime rotation of the allow-list (mirrors.json in
frontend/src-tauri/resources/mirrors.json) without a release, or (B) require app
updates to rotate mirrors. Locate and update the two contradictory statements
(the sentence asserting "rotatable without release" and the sentence stating
"rotation happens only via app updates") so they both state the chosen policy,
and update any dependent wording (e.g., architecture/requirements references) to
match.
Comment on lines
+324
to
+325
| from engines._subprocess import SubprocessBackend | ||
|
|
Contributor
There was a problem hiding this comment.
SubprocessBackend import path is inconsistent with Phase 2 contracts.
This example uses engines._subprocess, but Phase 2 artifacts define the primitive in backend/services/subprocess_backend.py. Keep one canonical path to prevent implementation drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
@.planning/phases/03-supertonic-3-engine-installer-mirror-reliability/03-RESEARCH.md
around lines 324 - 325, The import for SubprocessBackend is using the
non-canonical module path (engines._subprocess); change it to import the
primitive from the canonical Phase 2 location by importing SubprocessBackend
from backend.services.subprocess_backend so all references use the same module
(SubprocessBackend) and avoid implementation drift.
| <task type="auto"> | ||
| <name>Task 1: Pre-tag verification + cut v0.3.0-rc1</name> | ||
| <files>(no file modifications — only git tag and gh API calls)</files> | ||
| <action>Run all pre-tag gates from `<interfaces>` in order: (1) `git status` (clean working tree on main); (2) `git fetch origin && git rev-list --left-right --count main...origin/main` (no divergence); (3) `uv run pytest tests/test_version_consistency.py -v` (must pass); (4) `gh run list --workflow=ci.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success`); (5) `gh run list --workflow=release.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success` from the most recent merged PR's dry-run via 06-02); (6) verify `gh release view v0.3.0-rc1 2>&1 | grep -q "release not found"` (rc1 not yet cut). If any gate fails, STOP and surface the failure. If all pass, cut rc1: `git tag -a v0.3.0-rc1 -m "v0.3.0-rc1: Stabilization milestone release candidate 1"` then `git push origin v0.3.0-rc1`. Watch the run: `gh run watch` (blocks until the workflow completes). After release.yml succeeds across all 3 matrix legs (macOS-14, windows-2022, ubuntu-22.04 per release.yml:122-145), mark the rc1 GH Release as pre-release: `gh release edit v0.3.0-rc1 --prerelease`. Capture the release URL, draft status, and SHA256SUMS-*.txt attachment list — write them as a section header in `.planning/release/v0.3.0-runbook.md` under a new "rc1 cut record" subsection so the next task has a clean handoff.</action> |
Contributor
There was a problem hiding this comment.
Additional XML escape in Task 1 action block.
Line 181 contains 2>&1 in the stderr redirection pattern, which should be 2>&1. This is part of the command: gh release view v0.3.0-rc1 2>&1 | grep -q "release not found".
🐛 Proposed fix
Within the <action> block at line 181, locate and replace:
-verify `gh release view v0.3.0-rc1 2>&1 | grep -q "release not found"`
+verify `gh release view v0.3.0-rc1 2>&1 | grep -q "release not found"`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <action>Run all pre-tag gates from `<interfaces>` in order: (1) `git status` (clean working tree on main); (2) `git fetch origin && git rev-list --left-right --count main...origin/main` (no divergence); (3) `uv run pytest tests/test_version_consistency.py -v` (must pass); (4) `gh run list --workflow=ci.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success`); (5) `gh run list --workflow=release.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success` from the most recent merged PR's dry-run via 06-02); (6) verify `gh release view v0.3.0-rc1 2>&1 | grep -q "release not found"` (rc1 not yet cut). If any gate fails, STOP and surface the failure. If all pass, cut rc1: `git tag -a v0.3.0-rc1 -m "v0.3.0-rc1: Stabilization milestone release candidate 1"` then `git push origin v0.3.0-rc1`. Watch the run: `gh run watch` (blocks until the workflow completes). After release.yml succeeds across all 3 matrix legs (macOS-14, windows-2022, ubuntu-22.04 per release.yml:122-145), mark the rc1 GH Release as pre-release: `gh release edit v0.3.0-rc1 --prerelease`. Capture the release URL, draft status, and SHA256SUMS-*.txt attachment list — write them as a section header in `.planning/release/v0.3.0-runbook.md` under a new "rc1 cut record" subsection so the next task has a clean handoff.</action> | |
| <action>Run all pre-tag gates from `<interfaces>` in order: (1) `git status` (clean working tree on main); (2) `git fetch origin && git rev-list --left-right --count main...origin/main` (no divergence); (3) `uv run pytest tests/test_version_consistency.py -v` (must pass); (4) `gh run list --workflow=ci.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success`); (5) `gh run list --workflow=release.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success` from the most recent merged PR's dry-run via 06-02); (6) verify `gh release view v0.3.0-rc1 2>&1 | grep -q "release not found"` (rc1 not yet cut). If any gate fails, STOP and surface the failure. If all pass, cut rc1: `git tag -a v0.3.0-rc1 -m "v0.3.0-rc1: Stabilization milestone release candidate 1"` then `git push origin v0.3.0-rc1`. Watch the run: `gh run watch` (blocks until the workflow completes). After release.yml succeeds across all 3 matrix legs (macOS-14, windows-2022, ubuntu-22.04 per release.yml:122-145), mark the rc1 GH Release as pre-release: `gh release edit v0.3.0-rc1 --prerelease`. Capture the release URL, draft status, and SHA256SUMS-*.txt attachment list — write them as a section header in `.planning/release/v0.3.0-runbook.md` under a new "rc1 cut record" subsection so the next task has a clean handoff.</action> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.planning/phases/06-release-verification-retro/06-04-PLAN.md at line 181,
The stderr redirection in the `<action>` command contains an XML-escaped
ampersand (`2>&1`) which breaks the shell redirection; update the action
text that runs `gh release view v0.3.0-rc1 2>&1 | grep -q "release not
found"` to use the correct shell token `2>&1` (i.e., replace `2>&1` with
`2>&1`) so the pipeline `gh release view v0.3.0-rc1 2>&1 | grep -q "release not
found"` works as intended.
| <files>(no file modifications — only git tag and gh API calls)</files> | ||
| <action>Run all pre-tag gates from `<interfaces>` in order: (1) `git status` (clean working tree on main); (2) `git fetch origin && git rev-list --left-right --count main...origin/main` (no divergence); (3) `uv run pytest tests/test_version_consistency.py -v` (must pass); (4) `gh run list --workflow=ci.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success`); (5) `gh run list --workflow=release.yml --limit 1 --branch main --json conclusion --jq '.[0].conclusion'` (must equal `success` from the most recent merged PR's dry-run via 06-02); (6) verify `gh release view v0.3.0-rc1 2>&1 | grep -q "release not found"` (rc1 not yet cut). If any gate fails, STOP and surface the failure. If all pass, cut rc1: `git tag -a v0.3.0-rc1 -m "v0.3.0-rc1: Stabilization milestone release candidate 1"` then `git push origin v0.3.0-rc1`. Watch the run: `gh run watch` (blocks until the workflow completes). After release.yml succeeds across all 3 matrix legs (macOS-14, windows-2022, ubuntu-22.04 per release.yml:122-145), mark the rc1 GH Release as pre-release: `gh release edit v0.3.0-rc1 --prerelease`. Capture the release URL, draft status, and SHA256SUMS-*.txt attachment list — write them as a section header in `.planning/release/v0.3.0-runbook.md` under a new "rc1 cut record" subsection so the next task has a clean handoff.</action> | ||
| <verify> | ||
| <automated>gh release view v0.3.0-rc1 --json tagName,isDraft,isPrerelease,assets --jq '{tag:.tagName,draft:.isDraft,prerelease:.isPrerelease,assetCount:(.assets|length)}' && gh run list --workflow=release.yml --branch main --limit 1 --json conclusion --jq '.[0].conclusion'</automated> |
Contributor
There was a problem hiding this comment.
Critical: XML escape sequences will break shell commands.
The <verify><automated> blocks contain && and & instead of && and &, which are XML/HTML escape sequences. When these commands execute in a shell context, they will fail with syntax errors because the shell will interpret the literal string && rather than the logical AND operator &&.
Affected locations:
- Task 1 verification (Line 183)
- Task 4 verification (Line 234)
- Task 5 verification (Line 244)
- Task 7 verification (Line 283)
- Final verification section (Lines 315-318)
🐛 Proposed fix: Replace XML escapes with shell operators
Line 183 (Task 1):
- <automated>gh release view v0.3.0-rc1 --json tagName,isDraft,isPrerelease,assets --jq '{tag:.tagName,draft:.isDraft,prerelease:.isPrerelease,assetCount:(.assets|length)}' && gh run list --workflow=release.yml --branch main --limit 1 --json conclusion --jq '.[0].conclusion'</automated>
+ <automated>gh release view v0.3.0-rc1 --json tagName,isDraft,isPrerelease,assets --jq '{tag:.tagName,draft:.isDraft,prerelease:.isPrerelease,assetCount:(.assets|length)}' && gh run list --workflow=release.yml --branch main --limit 1 --json conclusion --jq '.[0].conclusion'</automated>Line 234 (Task 4):
- <automated>RELEASE_GATE=1 uv run pytest tests/test_changelog.py -v && gh release view v0.3.0 --json tagName,isDraft,isPrerelease,assets,body --jq '{tag:.tagName,draft:.isDraft,pre:.isPrerelease,assetCount:(.assets|length),bodyLen:(.body|length),hasFallback:(.body|test("Auto-generated release"))}'</automated>
+ <automated>RELEASE_GATE=1 uv run pytest tests/test_changelog.py -v && gh release view v0.3.0 --json tagName,isDraft,isPrerelease,assets,body --jq '{tag:.tagName,draft:.isDraft,pre:.isPrerelease,assetCount:(.assets|length),bodyLen:(.body|length),hasFallback:(.body|test("Auto-generated release"))}'</automated>Line 244 (Task 5):
- <automated>gh issue list --label deferred-from-v0.3 --milestone v0.4 --state open --json number --jq 'length' && awk '/^## \[0\.3\.0\]/{flag=1;next}/^## \[/{flag=0}flag' CHANGELOG.md | grep -cE "^\| #(35|42|48|54|55|56|57|58|60|65|72|76|80) " && for n in 35 42 48 54 55 56 57 58 60 65 72 76 80; do echo -n "$n:"; gh issue view $n --json state --jq .state; done</automated>
+ <automated>gh issue list --label deferred-from-v0.3 --milestone v0.4 --state open --json number --jq 'length' && awk '/^## \[0\.3\.0\]/{flag=1;next}/^## \[/{flag=0}flag' CHANGELOG.md | grep -cE "^\| #(35|42|48|54|55|56|57|58|60|65|72|76|80) " && for n in 35 42 48 54 55 56 57 58 60 65 72 76 80; do echo -n "$n:"; gh issue view $n --json state --jq .state; done</automated>Line 283 (Task 7):
- <automated>test -f .planning/retros/v0.3.0-retro.md && wc -l .planning/retros/v0.3.0-retro.md && grep -c "^## " .planning/retros/v0.3.0-retro.md && grep -cE "Weighted closure|Net inbox|Discord support" .planning/retros/v0.3.0-retro.md</automated>
+ <automated>test -f .planning/retros/v0.3.0-retro.md && wc -l .planning/retros/v0.3.0-retro.md && grep -c "^## " .planning/retros/v0.3.0-retro.md && grep -cE "Weighted closure|Net inbox|Discord support" .planning/retros/v0.3.0-retro.md</automated>Lines 315-318 (Verification section):
- `gh release view v0.3.0 --json tagName,isDraft,isPrerelease,assets --jq '{tag:.tagName,draft:.isDraft,pre:.isPrerelease,assetCount:(.assets|length)}'` => `{tag:"v0.3.0", draft:false, pre:false, assetCount: >= 8}`
- `RELEASE_GATE=1 uv run pytest tests/test_changelog.py -v` => pass
- `grep -E "^## \[0\.3\.0\] — 2[0-9]{3}-[0-9]{2}-[0-9]{2}$" CHANGELOG.md` => 1 match
- `gh issue list --label deferred-from-v0.3 --milestone v0.4 --state open --json number --jq 'length'` => 3
- For each of `35 42 48 54 55 56 57 58 60 65 72 76 80`: `gh issue view <n> --json state,milestone` shows CLOSED or v0.4-milestone
-- `.planning/retros/v0.3.0-retro.md` exists with all 6 sections and 3 metrics populated
-- `.planning/release/verification-matrix.md` "Wave-2 verifier signoff" column is filled for all rows(Note: Lines 319-320 don't contain shell commands with &&, so no changes needed there.)
Also applies to: 234-234, 244-244, 283-283, 315-318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.planning/phases/06-release-verification-retro/06-04-PLAN.md at line 183,
Several <verify><automated> XML blocks contain HTML-escaped shell operators
(&& and &) which will break when executed; open each
<verify><automated> block (the ones used for Task 1, Task 4, Task 5, Task 7 and
the final verification) and replace the escaped sequences && and &
with the actual shell operators && and & in the GH CLI commands (e.g., the gh
release view ... and gh run list ... command lines) so the commands run
correctly in a shell.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Planning artifacts for the v0.3.0 single-fat-milestone release. Docs-only — no production code changes.
Summary
This PR lands the 18 PLAN.md files + 6 RESEARCH.md files + 2 SPIKE ADRs + supertonic version bump that decompose v0.3.0 across the 7-phase roadmap. Output of a council-of-6-experts session yesterday (Critic, Strategist, User Advocate, Maintainer, Architect, Whisperer + Release Engineer) and 5 parallel phase researchers.
What lands
Scope decisions locked
Validation
gsd-sdk verify.plan-structure(Phase 4's 2 plans need<action>blocks on checkpoint tasks — fix lives in companion P0 PR).planning/decisions/No code touched
git diff --stat origin/main..HEADshows only.planning/**+CLAUDE.mdfiles. No backend/frontend/test changes.Companion PR
A separate PR will land the P0 security/correctness wave-1 code work (loopback guard, atomic WAV writes, ffmpeg pin, torch.load removal, TTSBackend.unload foundation, Phase 4 plan
<action>fix, capture_ws test fix). That PR depends on this PR landing first (the Phase 4 plan fix references plans introduced here).🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation