fix(responses): expose continuation spill write health (carry of #3525) - #3542
Conversation
Co-authored-by: Ingwannu <186453546+Ingwannu@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe change adds privacy-safe spill-write health metrics to response-state diagnostics. Spill paths record classified failures and recovery timestamps. The authenticated memory endpoint and related documentation expose the new fields, with tests covering initial, degraded, and recovered states. ChangesSpill-write health diagnostics
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The authenticated memory diagnostics expose the new spill-write health data without changing liveness behavior, and the covered status and recovery paths are ready to merge. Sequence Diagram(s)sequenceDiagram
participant SpillWritePaths
participant ResponseStateMetrics
participant MemoryEndpoint
SpillWritePaths->>ResponseStateMetrics: Record classified spill-write result
ResponseStateMetrics->>MemoryEndpoint: Provide health status and timestamps
MemoryEndpoint-->>MemoryEndpoint: Return authenticated memory diagnostics
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 70 / 80설명 이 PR은 이슈 #3522에서 보이는 Windows continuation spill 쓰기 실패가 왜 중요한지 한 줄로 말하면, continuation을 디스크에 넘기는 spill writer가 ACL 타임아웃·권한·용량 때문에 반복 실패해도 코드 경로를 현재 라인 수준에서 보면 설계 선택은 대체로 안전합니다. 성공해도 tests/server/memory-watchdog.test.ts - 주석이 “17 after #3522 added”라고 이슈 번호를 적었는데, 실제 추가 PR은 #3525/#3542입니다. 이슈와 PR을 섞어 쓰면 나중에 추적할 때 헷갈립니다. src/responses/state.ts noteSpillWriteFailure / classifySpillWriteFailure - docs-site/.../windows-memory.md - spill-health는 API에 있다고 잘 적었고, 대시보드 카드는 “memory and continuation-size fields”만 그린다고 범위를 좁혀 두었습니다. 운영자가 UI만 보면 새 필드를 못 볼 수 있으니, 머지 후 대시보드 노출은 별도 후속인지 한 줄로 적어 두면 더 좋습니다. #3525 - 이 캐리가 머지되면 원본 #3525는 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Summary
Continuation spill write failures accumulated silently behind a healthy readiness signal (#3522). This carry tracks spill write health in
src/responses/state.tsas a privacy-safe failure-class enum and exposes it through the management system routes, distinguishing an active failure streak from a recovered one. Instrumentation only — #3522 stays open per the original author.Supersedes #3525 (maintainer carry: GitHub reports the original CONFLICTING after the
tests/<domain>/migration, whilegit merge-treeis clean; the carry is the PR head merged withorigin/dev, no source edits).Stack (wp1 merge train, independent layers — each targets
dev):Unit:
devlog/_plan/260905_open_work_closeout/(010, 011).Verification
bun run typecheck— exit 0 on the carry head.bun test tests/server/memory-watchdog.test.ts— tests/server/memory-watchdog.test.ts — 13 pass / 0 fail (six tests RED on dev before the change).3525's exact head was full-matrix green; exact-head CI on this branch is the merge gate.Checklist
devCo-authored-bytrailerCo-authored-by: Ingwannu 186453546+Ingwannu@users.noreply.github.com
Summary by CodeRabbit
New Features
ocx observe memory --jsonCLI command.Documentation