Skip to content

wip connection tester updated - #1640

Open
lebaudantoine wants to merge 1 commit into
mainfrom
integration/test-connection-v2
Open

lebaudantoine wants to merge 1 commit into
mainfrom
integration/test-connection-v2

Conversation

@lebaudantoine

Copy link
Copy Markdown
Collaborator

No description provided.

@sonarqubecloud

Copy link
Copy Markdown

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Warn when connection tests use non-UDP media routes

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Detect successful ICE routes that fall back from UDP to TCP or TLS.
• Surface a localized warning with network requirements documentation for IT teams.
• Preserve failure precedence while carrying ICE report data into summary state.
Diagram

graph TD
  ICE["ICE Check"] -->|candidate report| Runner["Test Runner"] -->|step data| Route["Route Classifier"] -->|warning flag| Summary["Test Summary"] -->|help link| Docs["Network Docs"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. First-class warning step status
  • ➕ Models degraded outcomes consistently across future diagnostic checks
  • ➕ Could centralize warning counts, precedence, reporting, and presentation
  • ➖ Requires broader changes to status enums, statistics, rows, reports, and translations
  • ➖ May blur the distinction between a successful check and a degraded route

Recommendation: Keep the focused route-warning flag for this single diagnostic condition: it preserves the selected-candidate check as successful while clearly degrading the overall summary. A first-class warning status is worth adopting only if additional checks need non-failing warning outcomes; targeted tests for transport classification and summary precedence should accompany this approach.

Files changed (8) +89 / -7

Enhancement (4) +77 / -7
selectedCandidate.tsClassify selected non-UDP ICE routes as suboptimal +17/-0

Classify selected non-UDP ICE routes as suboptimal

• Adds a defensive helper that reads the selected candidate's relay or direct transport and flags known non-UDP routes. Missing or malformed diagnostic data remains non-warning.

src/frontend/src/features/diagnostics/checks/selectedCandidate.ts

ConnectionTestSummary.tsxDisplay degraded-route warnings and IT guidance +44/-6

Display degraded-route warnings and IT guidance

• Adds a warning summary state, warning styling, and a localized link to network requirements documentation. Failure results retain precedence over warnings, while warnings take precedence over partial results.

src/frontend/src/features/diagnostics/components/ConnectionTestSummary.tsx

useConnectionTestRunner.tsPreserve diagnostic check payloads in step results +3/-0

Preserve diagnostic check payloads in step results

• Copies LiveKit CheckInfo data into connection-test step results so the selected ICE candidate report remains available to the route UI.

src/frontend/src/features/diagnostics/hooks/useConnectionTestRunner.ts

ConnectionTest.tsxDerive summary warnings from selected ICE transport +13/-1

Derive summary warnings from selected ICE transport

• Inspects a successful selected-candidate step for non-UDP transport and passes the resulting warning flag into the connection summary.

src/frontend/src/features/diagnostics/routes/ConnectionTest.tsx

Other (4) +12 / -0
connectionTest.jsonAdd German degraded-connection messaging +3/-0

Add German degraded-connection messaging

• Adds German warning headline, quality-impact guidance, and IT documentation link text for non-UDP media routes.

src/frontend/src/locales/de/connectionTest.json

connectionTest.jsonAdd English degraded-connection messaging +3/-0

Add English degraded-connection messaging

• Adds English warning headline, quality-impact guidance, and IT documentation link text for non-UDP media routes.

src/frontend/src/locales/en/connectionTest.json

connectionTest.jsonAdd French degraded-connection messaging +3/-0

Add French degraded-connection messaging

• Adds French warning headline, quality-impact guidance, and IT documentation link text for non-UDP media routes.

src/frontend/src/locales/fr/connectionTest.json

connectionTest.jsonAdd Dutch degraded-connection messaging +3/-0

Add Dutch degraded-connection messaging

• Adds Dutch warning headline, quality-impact guidance, and IT documentation link text for non-UDP media routes.

src/frontend/src/locales/nl/connectionTest.json

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code failure identified.

The selected-candidate report supplies the route shape consumed by the new classifier, warning state precedence is explicit, translations remain aligned across supported locales, and run resets prevent route data from leaking across diagnostic attempts.

Important Files Changed

Filename Overview
src/frontend/src/features/diagnostics/checks/selectedCandidate.ts Adds route classification using the selected pair’s relay or first-hop transport; the produced report shape supplies the nested fields used by the helper.
src/frontend/src/features/diagnostics/hooks/useConnectionTestRunner.ts Propagates LiveKit check data into step results while existing run initialization and abort checks prevent stale asynchronous results.
src/frontend/src/features/diagnostics/routes/ConnectionTest.tsx Derives the warning only from a successful selected-candidate step in the current run.
src/frontend/src/features/diagnostics/components/ConnectionTestSummary.tsx Adds a non-UDP warning state below hard failures and above partial results, with localized guidance and documentation.
src/frontend/src/locales/en/connectionTest.json Adds the English warning headline, explanation, and documentation-link label alongside matching entries in every supported locale.

Reviews (1): Last reviewed commit: "wip connection tester updated" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant