update flink statement warnings UI based on new response structure#3254
update flink statement warnings UI based on new response structure#3254Noel Cothren (noeldevelops) wants to merge 22 commits intomainfrom
Conversation
741864d to
99f59ae
Compare
Clarifies that the type assertion should be removed once the OpenAPI spec includes the warnings field on statement status. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove hasWarnings, hasCriticalWarnings, and formattedWarnings signals that were not referenced in the template or elsewhere. Inline the formatting logic directly into the detailItems derivation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge identical .detail-item.error/.detail-item.critical and .detail-severity.error/.detail-severity.critical rules into combined selectors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
detailText was only consumed by detailItems, making it an unnecessary intermediate signal. Inline the logic to simplify the reactive graph. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4660052 to
22ba7e8
Compare
There was a problem hiding this comment.
Pull request overview
Updates the Flink Statement Results webview to surface statement warnings using the new structured warnings field from the getSqlv1Statement response, while remaining backward compatible with legacy [Warning] text embedded in status.detail.
Changes:
- Add warning parsing utilities to prefer structured API warnings and fall back to legacy
[Warning]parsing. - Extend statement models/results manager/webview view model to pass warnings through to the results viewer.
- Refactor results viewer “details/warnings” header UI into a unified list with updated HTML/CSS rendering.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/webview/flink-statement-results.ts | Builds a unified detailItems list combining error/info detail text with structured/legacy warnings. |
| src/webview/flink-statement-results.html | Reworks the header details UI to render multiple detail items with severity styling. |
| src/models/flinkStatement.ts | Adds a warnings getter that extracts structured warnings or parses legacy detail warnings. |
| src/flinkSql/warningParser.ts | New helper module to parse legacy warnings, prefer structured warnings, and strip [Warning] blocks from detail. |
| src/flinkSql/warningParser.test.ts | Unit tests for warning parsing/extraction/stripping behavior. |
| src/flinkSql/flinkStatementResultsManager.ts | Includes statement warnings in the data returned to the webview. |
| src/flinkSql/flinkStatementResultsManager.test.ts | Updates expected statement meta to include warnings. |
You can also share your feedback on Copilot code review. Take the survey.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if (rawDetail) { | ||
| const hasApiWarnings = meta.warnings.length > 0; | ||
| const processedDetail = hasApiWarnings ? stripWarningsFromDetail(rawDetail) : rawDetail; | ||
| detail = processedDetail ? processedDetail.replace(/\n/g, "<br>") : null; |
There was a problem hiding this comment.
Bug: This replaces newlines with literal <br> HTML tags, but the template now uses data-text (not data-html), so users will see the string <br> rendered as visible text. Since .detail-message has white-space: pre-wrap, newlines are already preserved — this .replace() call should be removed.
src/flinkSql/warningParser.ts
Outdated
| if (!detail) { | ||
| return null; | ||
| } | ||
| const warningPattern = /\[Warning\]\s*[\s\S]*?(?=\s*\[Warning\]|$)/gi; |
There was a problem hiding this comment.
DRY: This regex is a near-duplicate of the one in parseLegacyWarnings (line 28). Consider extracting a shared constant so both functions reference the same pattern.
| severity: w.severity, | ||
| message: w.message, | ||
| reason: w.reason, | ||
| createdAt: w.created_at ? String(w.created_at) : undefined, |
There was a problem hiding this comment.
Nit: String(w.created_at) on a Date produces the verbose format like Thu Nov 14 2025 16:01:00 GMT+0000 (Coordinated Universal Time). Consider using .toLocaleString() or .toISOString() for a cleaner timestamp display.
…ate format - Remove .replace(/\n/g, "<br>") since data-text + pre-wrap handles newlines - Extract duplicated legacy warning regex into shared LEGACY_WARNING_PATTERN - Use .toLocaleString() instead of String() for warning timestamps Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|


Summary of Changes
Click-testing instructions
Changes are visible in the VS Code Flink Statement Results viewer when there are Statement Details, Errors, or Warnings. There are detailed testing scenarios for the internal team in shared docs.
To test the new warnings API, check this branch out locally and change the
.envfile variable to point to Staging, where the feature flag is enabled. You will need to have an account on Staging with Flink compute pool running.IDE_SIDECAR_CONNECTIONS_CCLOUD_BASE_PATH=stag.cpdev.cloudflink-statement-results.tsfile. Setthis.hasWarningsto true & add an array of mocks e.g.Optional: Any additional details or context that should be provided?
Pull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes