Repository navigation
Conversation
Treated missing publish and pull evidence as unavailable rather than an empty clean report. Preserved incomplete-server warnings on mixed diagnostics and added regressions for diagnostics and write feedback. Fixes #15408
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. |
|
The review summary says the code review is still running and contains no finding or requested change. Is there a specific change you want on #15412, or should I wait for the review findings? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9a2e6847f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| server: serverNames.join(", "), | ||
| messages: [], | ||
| summary: "OK", | ||
| messages: incomplete ? [incomplete] : [], |
There was a problem hiding this comment.
Keep availability warnings out of diagnostic deduplication
When lsp.diagnosticsDeduplicate is left at its default and one server succeeds while another repeatedly times out, this availability status is treated as an ordinary diagnostic message. DiagnosticsLedger.reduce() records it on the first write, removes it on the next, and recomputes the result as no issues; deferred delivery even drops the result entirely when no messages remain. Thus subsequent writes can again appear clean despite the server still supplying no report. Preserve incomplete-server state separately from deduplicated diagnostics and cover the repeated-report transition.
AGENTS.md reference: AGENTS.md:L308-L308
Useful? React with 👍 / 👎.
Separated availability status from diagnostic identities so repeated writes and deferred delivery never turn an incomplete report into a clean one. Preserved the status through batched write aggregation and covered repeated reports. Fixes #15408
|
Addressed the repeated-report regression in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdd910de82
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const formatted = uniqueDiagnostics.map(d => formatDiagnostic(d, relPath)); | ||
| const limited = limitDiagnosticMessages(formatted); | ||
| const summary = formatDiagnosticsSummary(uniqueDiagnostics); | ||
| if (incomplete) limited.push(incomplete); |
There was a problem hiding this comment.
Surface availability warnings outside capped diagnostic rows
When a successful server returns at least five diagnostics while another server fails, appending incomplete after the diagnostic messages hides the availability warning in the native write/edit UI: diagnosticsSection() groups parsed diagnostics before unparsed messages and displays only its first five rows, while that native render path does not show summary or otherwise use unavailable. The user therefore sees ordinary diagnostics plus “+N more” without learning that the report is incomplete; render unavailable as a separate status row outside the diagnostic cap.
Useful? React with 👍 / 👎.
Rendered the incomplete-report warning as a separate notice row after the capped diagnostic rows so five or more diagnostics can no longer hide it. Carried the warning through deferred late-diagnostics delivery into native edit/write frames. Fixes #15408
|
Addressed in
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3ac18cd4d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| server: serverNames.join(", "), | ||
| messages: [], | ||
| summary: "OK", | ||
| messages: incomplete ? [incomplete] : [], |
There was a problem hiding this comment.
Exclude availability statuses from diagnostic counts
When one server reports a clean file while another server fails, this produces messages: [incomplete] even though there are zero diagnostics. The native write/edit header calls diagnosticsBadge(), which counts file.messages.length, so it displays “1 diagnostic”; reports containing actual diagnostics are similarly inflated by one. Keep the availability status outside the counted message list or teach the badge to exclude unavailable.
Useful? React with 👍 / 👎.
Repro
A push-only LSP server that never publishes for a document makes
lsp diagnosticsreportOKwithsuccess: trueand makes write feedback claimsummary: "OK". Reproduce the wait path before the fix withbun -e 'import { waitForDiagnostics } from "./packages/coding-agent/src/lsp/diagnostics.ts"; const client = { name:"silent-push", diagnostics:new Map(), diagnosticsVersion:0, openFiles:new Map(), serverCapabilities:{} }; console.log(await waitForDiagnostics(client, "file:///tmp/broken.toml", {timeoutMs:60, minVersion:0}));'(printed[]).Cause
packages/coding-agent/src/lsp/diagnostics.tswaitForDiagnostics()returned[]after its deadline when no fresh publish or advertised pull was observed.LspTool.execute()interpreted that as a verified clean document;getDiagnosticsForFile()likewise produced a clean write result.Fix
lsp diagnostics, write feedback, and mixed-server output in regression tests; document the user-visible change.Verification
bun test packages/coding-agent/test/tools/lsp-diagnostics-freshness.test.ts packages/coding-agent/test/tools/lsp-regressions.test.ts: 152 passed.bun run check:ts: passed. A post-fixbun -esmoke of the silent-server wait printedUNVERIFIED: No fresh diagnostics published or pulled within 60ms. Generated the missingtool-views.generated.jsasset after an initial full-suite failure; startup probes then passed. Skipped the PR pre-publish gate:bun run testfails on a cleanorigin/mainsnapshot forpackages/coding-agent/test/utils/changelog.test.ts(interactive PTY startup emits 678487 bytes, exceeding the 524288-byte ceiling); the same test on this branch emitted 677881 bytes and failed identically. The branch push pre-publish gate passed.Fixes #15408