wp3: land four small contributor changes and the missing feedback-callback test - #4053
Conversation
Carried from #3980 (author yansigit) for maintainer-side CI and sequential integration. Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>
Carried from #3963 (author luvs01) for maintainer-side CI and sequential integration. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Carried from #3984 (author yansigit) for maintainer-side CI and sequential integration. Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>
#3984 changes publishFeedback to a useCallback and adds it to saveDisplayName's dependency array, but shipped without coverage, so the hygiene gate flags missing_regression_test. This source-oracle test asserts both halves and fails on dev without the fix (0 pass / 2 fail), so the suppression cannot come back unnoticed. Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
documentation
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (29)
📒 Files selected for processing (45)
💤 Files with no reviewable changes (32)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change removes dashboard evidence assets, extracts API-key selection capture from the stateful provider module, stabilizes a GUI feedback callback, and updates CLI status tests to allocate isolated ephemeral ports. ChangesDashboard evidence cleanup
Router selection capture extraction
GUI feedback callback stability
CLI test port isolation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change removes historical dashboard captures, makes CLI status tests use isolated ports, stabilizes feedback callbacks, and extracts API-key snapshot capture without changing its public export or selection behavior. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The PR includes substantial changes unrelated to linked issue Resolution Split the stale-port, dashboard-asset, and feedback-callback changes into separate pull requests with their relevant issue links, or link the corresponding issues to this PR. Keep this PR limited to the API-key selection extraction, compatibility export, router import update, related documentation, and focused provider tests. Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. (6 skipped: 6 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
리뷰 · 우선순위 70 / 80이 PR은 지금 네 조각(+테스트)이 지금 체크아웃 경로에서 하는 일을 풀어서 쓰면 이렇습니다. 첫째는 검증 쪽은 본문 기준 라인 711-798 / allocateFreePort - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
#3984 added its review screenshot under a new assets/pr-screenshots/ directory. The repository's nineteen existing PR screenshots all live in docs-site/public/pr-screenshots/, so this moves it there and keeps the new top-level directory out of the tree. Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>
|
Re-running the target gate: the two |
1 participant
Summary
Third sequential integration branch of the 2.49.0 backlog closeout. Four small contributor changes plus the regression test one of them was missing, carried as squash commits that each keep a
Co-authored-bytrailer. Stacked ondevafter #4051.beforeAllthat the later test's own listener could then reclaim, inverting what the case asserts. The record port is now allocated after the occupied bind rather than merely asserted to differ. Test-only.src/router.tsimported API-key selection capture from a module that imports the router back, the cycle reported in Remove the direct router and API-key-selection import cycle #3894. The capture is extracted into its own module with a compatibility re-export, so existing importers are unaffected. Ten lines of movement, no behavior change.devpoint at.mdfiles the change retains.publishFeedbackwas a plain function called from 21 sites including inside thesaveDisplayNamecallback, so it was a new identity every render andsaveDisplayNamehad to omit it from its dependency array. It becomes auseCallbackwith an honest empty dependency list, andsaveDisplayNamedeclares it.missing_regression_test.tests/gui/models-feedback-callback.test.tsasserts both halves of that fix as a source oracle, registered inscripts/test-layout/layout.jsonandtests/fixtures/test-layout-expected.json. Reverting onlyModels.tsxtodevturns it red (0 pass / 2 fail), so it is not vacuous.The sponsor pair #3914/#3915 is intentionally not in this branch; it stays deferred until the sponsorship is settled.
Closes #3894
GUI change
#3984 touches
gui/src/pages/Models.tsxonly to change a callback's identity — no markup, styling,copy, or layout changes, so the Models page renders exactly as before. The screenshot below is the
one @yansigit captured for the original PR and it ships in this branch at
docs-site/public/pr-screenshots/3984-model-feedback.png(moved there from the new top-level directory the original PR introduced, so it sits with the other nineteen):Verification
bun x tsc --noEmit— exit 0.bun teston the three changed test files — 56 pass / 0 fail / 292 expect().bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/gui/models-feedback-callback.test.ts— 19 pass / 0 fail.gui/src/pages/Models.tsxreverted todevit reports 0 pass / 2 fail; restored, 2 pass.bun run privacy:scan— passed.bun run skill:surface:check— current.git merge --squash, no conflicts.bun run test(hosted CI covers Linux, Windows, macOS).Checklist