docs: retire docs/superpowers, relocating two pieces of rationale first - #3628
Conversation
docs/superpowers/ is gitignored (.gitignore:90). All six files were force-added past it, against a policy set two days earlier. Four are shipped-work recipes or restatements of code that already exists with its own tests; two carried rationale that lived nowhere else. Relocate those two, then delete all six. Relocated: - Provider wait accounting -> docs/architecture/27-agent-message-stream-dataflow.md. Verified against src/agent/streaming/lifecycle/deadlines.ts rather than copied. Two refinements the spec text did not carry: resumeProviderWait refills to a full budget when the deadline *kind* changes, and a settled read only wins its tie when settledAtMs <= deadlineAbsMs. The rest holds as written, and deadlines.test.ts names a case for each claim. - Context OOM forensics -> a comment beside the proxy lock in scripts/build/compile-binary.ts. Chose code over docs/architecture: no architecture page covers release artifacts, so a new one would have no inbound link, while this comment sits exactly where the pressure to fold the proxy profile back onto the workspace lock lands. The file already documents the declarative-evaluator crash-loop the same way. Deleted outright: - plans/2026-07-24-enterprise-stream-lifecycle.md - says Gates 1-4 are complete and that its checkboxes "are not live status markers"; defers completion evidence to docs/internal/stream-lifecycle-rollout.md. - plans/2026-07-30-project-creation-module.md - recipe for cli/shared/project-creation.ts, which exists with its own tests. - plans/2026-08-03-proxy-memory-footprint.md - recipe whose Task 1 is "import files from draft PR #3280", merged as afa6e30 in v0.1.1191. - specs/2026-07-30-project-creation-design.md - interface block is a verbatim copy of cli/shared/project-creation.ts; both rejected alternatives are in commit cde7dd8's body. - specs/2026-07-24-stream-lifecycle-design.md and specs/2026-08-03-proxy-memory-footprint-design.md - after relocation. docs/internal/stream-lifecycle-rollout.md is deliberately untouched: VF_STREAM_LIFECYCLE_MODE still defaults to legacy, so that runbook is a live rollback procedure, not a record of finished work. No inbound references to any deleted file exist.
|
Warning Review limit reached
Next review available in: 19 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: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43b37f27ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… section Review follow-up on the provider wait accounting section. The "Telemetry cannot do this" sentence described the wrong execution path. When status wins, raceProviderRead only sets statusEmitted; the runner then calls pauseProviderWait, which clears deadlineAbsMs, and the next resumeProviderWait both schedules the following status time and re-arms deadlineAbsMs from the carried-over remainingMs. Saying that status emission schedules the next status and never touches deadlineAbsMs misleads anyone debugging backpressure: deadlineAbsMs is recomputed on every resume. The invariant is about the budget, so state it that way. Verified against src/agent/streaming/lifecycle/deadlines.ts:139-198 and src/agent/streaming/lifecycle/runner.ts:165-192. Also replaced both em dashes this PR introduced with ASCII punctuation. AGENTS.md bans them, and the two lines added here were the only em dashes in all 26 files of docs/architecture. Docs only, no behavior change.
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
No actionable findings.
docs/architecture/27-agent-message-stream-dataflow.md:332accurately describes the lifecycle behavior implemented insrc/agent/streaming/lifecycle/deadlines.ts:139andrunner.ts:163; tie ordering, paused provider budgets, and absolute attempt timeout handling all match code.- Existing focused coverage exercises the documented backpressure and cached-read paths:
deadlines.test.ts:52,deadlines.test.ts:91,deadlines.test.ts:151, andrunner.test.ts:153. scripts/build/compile-binary.ts:108changes rationale comments only; compile arguments and profile behavior remain unchanged. The proxy build contract remains covered byscripts/build/compile-binary.test.ts:179.- The deleted
docs/superpowers/artifacts were ignored by policy (.gitignore:90), had no remaining inbound references, and the active lifecycle rollback runbook remains atdocs/internal/stream-lifecycle-rollout.md:1.
Rubric: correctness 40/40, tests 20/20, reliability/security 15/15, maintainability 15/15, scope/docs 10/10.
Review-Gate:
Reviewer: Codex
Reviewed-SHA: f569924
Score: 100/100
Actionable-Findings: 0
Verdict: APPROVE
2 participants
docs/superpowers/is gitignored (.gitignore:90). All six files were force-added past it, against a policy set two days earlier. Four are shipped-work recipes or restatements of code that already exists with its own tests; two carried rationale that lived nowhere else. This relocates those two, then deletes all six.Relocated (verified against the code, not copied)
Provider wait accounting ->
docs/architecture/27-agent-message-stream-dataflow.mdChecked against
src/agent/streaming/lifecycle/deadlines.tsbefore writing. The spec's core claims hold: provider-wait clocks pause when a read settles and resume with the stored remaining duration; the absolute attempt limit and external cancellation deliberately do not pause under backpressure, because "the absolute limit bounds total wall-clock ownership of an attempt, not provider availability".Two refinements the spec text did not carry, now written as the code behaves:
resumeProviderWaitdoes refill to a full budget when the deadline kind changes (kind !== activeKind), so entering tool-input-idle starts its own budget rather than inheriting a partly consumed one. The spec's flat "does not grant a new full timeout budget" was incomplete.settledAtMs <= deadlineAbsMs. A read that landed after the deadline elapsed does not retroactively win. That guard is what actually makes the ordering deterministic; the spec asserted determinism without it.deadlines.test.tsnames a case for each claim — "pauses provider idle while a frame is held but keeps total attempt time", "resumes the remaining provider-wait budget after consumer backpressure", "discards a cached provider result when the attempt deadline wins".Context OOM forensics -> a comment beside the proxy lock in
scripts/build/compile-binary.tsThe sole record of why a dedicated proxy binary exists: the 887,756,293 -> 951,154,705 byte growth across v0.1.1185/v0.1.1186, commit 339367a adding the Sharp/LightningCSS/PurgeCSS includes, and why V8 old-space tuning does not help (a bare
--versionOOMs before any traffic, so the cost is the embedded dependency archive, not the JS heap).Chose the code comment over
docs/architecture/: no architecture page covers release artifacts (14-build-pipeline.mdis the user-project build), so a new page would have no inbound link and no readership. The comment instead sits exactly where the pressure to "simplify" the proxy profile back onto the workspace lock lands, and the file already documents the declarative-evaluator crash-loop in this same style.Deleted outright
plans/2026-07-24-enterprise-stream-lifecycle.mddocs/internal/stream-lifecycle-rollout.mdplans/2026-07-30-project-creation-module.mdcli/shared/project-creation.ts, which exists with its own testsplans/2026-08-03-proxy-memory-footprint.mdspecs/2026-07-30-project-creation-design.mdcli/shared/project-creation.ts; both rejected alternatives are already in commit cde7dd8's bodyspecs/2026-07-24-stream-lifecycle-design.mdspecs/2026-08-03-proxy-memory-footprint-design.mdDeliberately untouched
docs/internal/stream-lifecycle-rollout.mdstays.VF_STREAM_LIFECYCLE_MODEstill defaults tolegacy, so the rollout has not cleared its first gate and that file is a live rollback procedure — not a record of finished work. Two of the deleted files discuss the same subject; they are not the same document.Checks
docs/rfcs/0001-ui-primitive-adapters.md:291).deno fmt --checkclean (2.7.7).scripts/build/compile-binary.test.ts16 passed,tests/unit/build/compile-binary-includes.test.ts9 steps passed,src/agent/streaming/lifecycle/deadlines.test.ts4 steps passed.