chore: strengthen spec-review with liveness and promise ownership checks#175
Merged
Conversation
…afe integer checks Add structured checklist to spec-review covering: - Input validation: exact valid domain for numeric options (safe integer) - Concurrency safety AND liveness (idle slots keep polling, freed slots reused) - Detached promise rejection ownership (no unhandled rejections) - stop()/shutdown must use allSettled pattern - Behavioral regression criteria Sync matching items to spec-draft quality checklist. Learned from #173 CodeRabbit review: idle slot loss, unhandled rejection, and unsafe integer validation were missed at spec stage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
📝 Walkthroughウォークスルー品質チェックリストが更新され、新しいパブリックAPI数値オプションの汎用入力テスト要件が、count/concurrency/loop-boundパラメータの明示的なドメイン定義要件に置き換わりました。並行実行/スケジューリング変更に対する安全保証と活動保証が追加され、detached対trackedプロミスの拒否所有権が明示的に定義される要件が含まれました。 変更内容
推定コードレビュー工数🎯 2 (Simple) | ⏱️ ~10分 関連する可能性のあるPR
詩
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.takt/facets/instructions/spec-review.md (1)
29-31: 並行性と活性の要件が詳細で実用的です「Concurrency and scheduling」セクションは、安全性（重複なし、二重実行なし）と活性（アイドルリソースの継続的なポーリング、解放されたスロットの再利用、制限された遅延内での新しい作業のピックアップ）の両方をカバーしており、状態遷移ごとの明示的な基準（作業発見、部分的アイドル、完全アイドル、エラー/拒否、停止）を要求しています。これにより、issue
#173で特定されたアイドルスロット喪失の問題を防ぐことができます。ただし、31行目の「keeps polling」という表現は、静的解析ツールによりよりフォーマルな表現が推奨されています。必要に応じて「continues to poll」への変更を検討してください。
♻️ スタイル改善の提案
- - Require explicit criteria for each state transition: work found → immediate action, partially idle → keeps polling, fully idle → maintenance + delayed poll, error/reject → no orphaned state, stop → predictable drain. + - Require explicit criteria for each state transition: work found → immediate action, partially idle → continues to poll, fully idle → maintenance + delayed poll, error/reject → no orphaned state, stop → predictable drain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.takt/facets/instructions/spec-review.md around lines 29 - 31, Update the wording in the "Concurrency and scheduling" section: replace the informal phrase "keeps polling" with the more formal "continues to poll" (referencing the phrase inside the second bullet under **Concurrency and scheduling**) so the spec reads with a formal, static-analysis-friendly term; no behavior change needed, only the textual edit in that bullet.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.takt/facets/instructions/spec-review.md:
- Around line 29-31: Update the wording in the "Concurrency and scheduling"
section: replace the informal phrase "keeps polling" with the more formal
"continues to poll" (referencing the phrase inside the second bullet under
**Concurrency and scheduling**) so the spec reads with a formal,
static-analysis-friendly term; no behavior change needed, only the textual edit
in that bullet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d46b2cda-3c42-40ff-97a7-bca3fab21d81
📒 Files selected for processing (2)
.takt/facets/instructions/spec-draft.md.takt/facets/instructions/spec-review.md
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.
Summary
Expand spec-review instruction with structured checklist covering gaps found in #173:
Also synced matching items to spec-draft quality checklist.
Context
Post-mortem from #173: CodeRabbit caught 3 issues (idle slot loss, unhandled rejection, unsafe integer) that spec-review should have caught. Codex (gpt-5.4) confirmed these were preventable at spec stage with the right checklist items.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit