docs: full designs for Phase 3b (maintainability), 3c (QoL/DX), and 3d (power/performance) - #10
Conversation
…d (power/performance) Expands the three backlog outlines from the public-release-readiness roadmap into fully specified, independently implementable designs, each with its own spec file matching this repository's one-spec-per-phase convention: - Phase 3b (maintainability refactor): splits lsp/pyright.py (1064 lines, 3 of the repo's 9 C901 complexity hits) into a lsp/pyright/ subpackage along its five existing concerns, deletes the dead wrap_uv_result/wrap_ruff_result code per Phase 3a's Resolved Decision 2, extracts inspect_workspace_service into its own module, deduplicates the safe-diff-preview validator out of cli/ruff.py, and adds a scoped complexity gate plus a file-length CI guardrail. - Phase 3c (quality-of-life & DX): fixes audit logging being completely inert by default (empirically verified: WARNING level, zero handlers, every existing audit event silently dropped), adds --check-tools/ --selftest/--print-schema CLI commands, a test-enforced diagnostics/ decision reference table in the README, richer inspect_workspace config-override metadata, broader MCP-client quickstart coverage, and an explicit deferred-decision process for adding new validator tools. - Phase 3d (power & performance): parallelizes uv/Ruff/Pyright execution with a bounded thread pool, shared-deadline timeout accounting, and deterministic post-hoc response reordering; an opt-in streamable-http transport that fails closed without a mandatory bearer token; scoped informational-only project-layout detection; and explicit, reasoned decisions not to build response caching or incremental shadow-copy mechanisms now. Every proposed script/config snippet in Phase 3b was actually executed against this repository during self-review, which caught and fixed three real mistakes before they could reach an implementation plan: a git ls-files glob pathspec that silently skipped 22 of 32 tracked files, a file-length allowlist that incorrectly assumed it would be empty, and a complexity threshold that was off by one. Phase 3c and 3d claims about current behavior (logging silence, wheel contents, mcp SDK transport/auth capabilities) were each verified by running code or reading source in this session rather than assumed. Updates the public-release-readiness roadmap doc to link to all three new specs and replace its backlog-outline language accordingly. No source code changes; this is a brainstorming/design deliverable only. Co-authored-by: Qkal <qkal@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| - Ambiguity check: the `max-complexity = 12` threshold's rationale (lowest | ||
| value under which the three split-out functions pass without further | ||
| rewriting) is stated explicitly so a future reader cannot mistake it for | ||
| an arbitrary number. |
There was a problem hiding this comment.
🟡 Design spec's self-review note references the wrong complexity threshold, contradicting the rest of the document
The self-review ambiguity-check note states the threshold is 12 (max-complexity = 12 at docs/superpowers/specs/2026-06-30-agent-quality-mcp-maintainability-refactor-design.md:519) even though the document corrected this to 13 everywhere else (lines 234, 246, 256–262, 471), and even documents catching and fixing this exact mistake in a prior self-review note (lines 500–507).
Impact: An implementer reading the self-review notes may be confused about which threshold value is correct, since the "ambiguity check" note — whose purpose is to prevent exactly this confusion — itself contains the stale value.
Stale value left over from pre-fix draft
The self-review at lines 500-507 documents catching the 12 → 13 fix in the main body. But the later "Ambiguity check" bullet at line 519 still says max-complexity = 12. The correct value used in the Complexity Gate section (docs/superpowers/specs/2026-06-30-agent-quality-mcp-maintainability-refactor-design.md:234), the TOML example (docs/superpowers/specs/2026-06-30-agent-quality-mcp-maintainability-refactor-design.md:246), and the acceptance criteria (docs/superpowers/specs/2026-06-30-agent-quality-mcp-maintainability-refactor-design.md:471) is 13.
| - Ambiguity check: the `max-complexity = 12` threshold's rationale (lowest | |
| value under which the three split-out functions pass without further | |
| rewriting) is stated explicitly so a future reader cannot mistake it for | |
| an arbitrary number. | |
| - Ambiguity check: the `max-complexity = 13` threshold's rationale (lowest | |
| value under which the three split-out functions pass without further | |
| rewriting) is stated explicitly so a future reader cannot mistake it for | |
| an arbitrary number. |
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
Brainstorming/design deliverable only — no source code changes. Follow-up to the Public Release Readiness design, which left Phases 3b/3c/3d as backlog outlines. This PR expands each into its own fully specified, independently implementable design, per the maintainer's request to fully design "3c, 3d, and other similar ideas" — interpreted as all three remaining phases, since 3b (maintainability) was the very first thing requested in the original brainstorming ask.
Stacked on top of #9 (this branch builds on
cursor/public-release-readiness-design-48a4) since these specs reference and build on that roadmap doc; the diff here is scoped to just the three new specs plus the roadmap update.New documents
docs/superpowers/specs/2026-06-30-agent-quality-mcp-maintainability-refactor-design.md(Phase 3b)docs/superpowers/specs/2026-06-30-agent-quality-mcp-quality-of-life-dx-design.md(Phase 3c)docs/superpowers/specs/2026-06-30-agent-quality-mcp-power-performance-design.md(Phase 3d)docs/superpowers/specs/2026-06-30-agent-quality-mcp-public-release-readiness-design.md(roadmap doc now links to all three)Highlights per phase
Phase 3b — Maintainability: splits
lsp/pyright.py(1,064 lines; contains 3 of the repo's 9C901complexity hits) into alsp/pyright/subpackage along its five existing concerns; deletes the deadwrap_uv_result/wrap_ruff_resultcode per Phase 3a's Resolved Decision 2; extractsinspect_workspace_serviceinto its own module; dedupes the safe-diff-preview validator out ofcli/ruff.py; adds a scoped complexity gate and a file-length CI guardrail.Phase 3c — Quality-of-Life & DX: fixes audit logging being completely inert by default — empirically verified in this session (
logging.getLogger("agent_quality_mcp.audit").getEffectiveLevel()→30/ WARNING, zero handlers anywhere), so every existing audit event is silently dropped today. Adds--check-tools/--selftest/--print-schemaCLI commands, a test-enforced diagnostics/decision reference table in the README, richerinspect_workspaceconfig-override metadata, broader MCP-client quickstart coverage, and an explicit deferred-decision process for adding new validator tools (no new tool is chosen here).Phase 3d — Power & Performance: parallelizes
uv/Ruff/Pyright execution with a bounded thread pool, shared-deadline timeout accounting, and deterministic post-hoc response reordering (directly answering the original Pyright-LSP spec's deferral reasons rather than ignoring them); an opt-instreamable-httptransport that fails closed without a mandatory bearer token; scoped informational-only project-layout detection; and explicit, reasoned decisions not to build response caching or incremental shadow-copy mechanisms now, with the safety/correctness concerns recorded so they aren't silently re-proposed later.Verification performed during self-review
Every proposed script/config in Phase 3b was actually executed against this repository, not just described — which caught and fixed three real mistakes before they could reach an implementation plan:
git ls-filesglob pathspec that silently skipped 22 of 32 tracked Python files (missing:(glob)magic).patching.pyalso needed allowlisting, not justservice.py.max-complexitythreshold that was off by one (12instead of13) — verified by actually runningruff check --select C901with the proposed config.Phase 3c/3d claims about current behavior (logging silence, wheel contents excluding test fixtures,
mcpSDK transport/auth capabilities viaTokenVerifier/TransportSecuritySettings) were each verified by running code or reading source in this session rather than assumed.Hard gate
Per the brainstorming workflow, no implementation happens until each design is reviewed and approved — these are independent specs; the maintainer can approve/adjust each separately before any is turned into an implementation plan.
Test plan
git diff --checkwhitespace issues.