doc: Fix paths in RADAR OpenAPI definition#2806
Conversation
📝 WalkthroughWalkthroughUpdates the radar OpenAPI document to generate schemas from TypeBox, add shared security components, and rewrite radar, control, and target routes to relative paths. ChangesRadar OpenAPI updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/radar/openApi.ts (2)
381-383: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove the stale WebSocket URL promise from the listing description.
The current
RadarInfoschema no longer documents spoke/control WebSocket URL fields, but this description still says every entry includes them. As per path instructions, “Flag any comments, docs, or code that refer to something not present in the current state of the branch.”Proposed wording update
description: - 'Returns all radars detected on the network. Each entry includes WebSocket URLs for spoke data and control streams.', + 'Returns all radars detected on the network. Each entry contains radar identifiers and static metadata.',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/radar/openApi.ts` around lines 381 - 383, Update the OpenAPI listing description in the radar schema to remove the outdated promise that each radar entry includes spoke/control WebSocket URLs. Adjust the text in the response description near the RadarInfo documentation so it only describes the fields that are actually present now, keeping the wording aligned with the current schema and avoiding references to removed WebSocket URL properties.Source: Path instructions
464-519: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude
{radar_id}in the single-control path.Both operations declare
radarIdParam, but the path template is only/controls/{control_id}. This produces an invalid/misleading API contract and drops the radar context from generated clients.Proposed path fix
- '/controls/{control_id}': { + '/{radar_id}/controls/{control_id}': {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/radar/openApi.ts` around lines 464 - 519, The single-control OpenAPI path is missing the radar identifier even though both the GET and PUT operations already include radarIdParam, so the contract is inconsistent and misleading. Update the path definition in openApi.ts for the control endpoints to include the radar context in the route template, and make sure the existing parameters and operation docs for the control_id handlers match that updated path so generated clients see both identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/radar/openApi.ts`:
- Around line 49-58: Update the RadarInfo OpenAPI schema in openApi.ts to match
the shared radar contract used by radarapi.ts and radar-schemas.ts: replace the
maxSpokeLength property with maxSpokeLen, and stop requiring brand in the
required array so it remains optional. Keep the rest of the RadarInfo shape
aligned with the shared schema so generated clients use the same field names and
required fields as the runtime contract.
---
Outside diff comments:
In `@src/api/radar/openApi.ts`:
- Around line 381-383: Update the OpenAPI listing description in the radar
schema to remove the outdated promise that each radar entry includes
spoke/control WebSocket URLs. Adjust the text in the response description near
the RadarInfo documentation so it only describes the fields that are actually
present now, keeping the wording aligned with the current schema and avoiding
references to removed WebSocket URL properties.
- Around line 464-519: The single-control OpenAPI path is missing the radar
identifier even though both the GET and PUT operations already include
radarIdParam, so the contract is inconsistent and misleading. Update the path
definition in openApi.ts for the control endpoints to include the radar context
in the route template, and make sure the existing parameters and operation docs
for the control_id handlers match that updated path so generated clients see
both identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3467de16-169b-4755-8e76-2048dc2727a3
📒 Files selected for processing (1)
src/api/radar/openApi.ts
|
@keesverruijt I updated the OpenAPI def based on the actual results received from the API. |
|
Sounds good to me! |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/radar/openApi.ts (1)
376-422: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore
/{radar_id}in the single-control route.The detail route dropped the radar segment, but both methods still declare
radarIdParam. This makes the path template inconsistent with its parameters and generates the wrong URL for the radar-scoped control endpoints.Proposed fix
- '/controls/{control_id}': { + '/{radar_id}/controls/{control_id}': {This also aligns the endpoint with the PR objective of fixing the OpenAPI paths so “Try It” works correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/radar/openApi.ts` around lines 376 - 422, The single-control endpoint path is missing the radar scope, so update the route definition for the controls detail handler in openApi.ts to restore /{radar_id} alongside /controls/{control_id}. Keep the existing radarIdParam and control_id parameter declarations aligned with the path template in both the GET and PUT operations so the OpenAPI spec generates the correct radar-scoped URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/radar/openApi.ts`:
- Around line 19-21: The radar OpenAPI spec is still bypassing type safety by
declaring radarApiDoc as any and casting it back to OpenApiDescription. Update
the openApi.ts definition to use a proper structural OpenAPI type (or the
branded OpenApiDescription if it fits) so the spec is validated at compile time,
and remove the any escape hatch and related override in favor of types that
match the actual document shape.
---
Outside diff comments:
In `@src/api/radar/openApi.ts`:
- Around line 376-422: The single-control endpoint path is missing the radar
scope, so update the route definition for the controls detail handler in
openApi.ts to restore /{radar_id} alongside /controls/{control_id}. Keep the
existing radarIdParam and control_id parameter declarations aligned with the
path template in both the GET and PUT operations so the OpenAPI spec generates
the correct radar-scoped URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6cad08c-0c0e-4199-b325-05c7b824281f
📒 Files selected for processing (1)
src/api/radar/openApi.ts
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
|
|
||
| export const radarApiDoc: any = { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -p 'swagger.ts' src/api
echo
sed -n '1,220p' src/api/swagger.ts
echo
sed -n '19,30p;517,521p' src/api/radar/openApi.tsRepository: SignalK/signalk-server
Length of output: 6914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,80p' src/api/swagger.ts
echo
rg -n "export type Brand|type Brand|interface Brand" src node_modules/@signalk -g '*.ts' -g '*.d.ts'
echo
sed -n '1,80p' src/api/radar/openApi.tsRepository: SignalK/signalk-server
Length of output: 2835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "OpenApiDescription|as unknown as OpenApiDescription|export const .*ApiDoc: any" src/api
echo
sed -n '1,120p' src/api/radar/openApi.ts
echo
sed -n '500,525p' src/api/radar/openApi.tsRepository: SignalK/signalk-server
Length of output: 6554
Remove the any escape hatch from the radar OpenAPI spec. radarApiDoc is still built as any and cast back to OpenApiDescription, so this file bypasses the type checks that should catch OpenAPI drift. If the branded OpenApiDescription is too narrow, replace it with a structural OpenAPI type instead of opting out here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/radar/openApi.ts` around lines 19 - 21, The radar OpenAPI spec is
still bypassing type safety by declaring radarApiDoc as any and casting it back
to OpenApiDescription. Update the openApi.ts definition to use a proper
structural OpenAPI type (or the branded OpenApiDescription if it fits) so the
spec is validated at compile time, and remove the any escape hatch and related
override in favor of types that match the actual document shape.
Source: Coding guidelines
|
@tkurki @keesverruijt Have updated to use TypeBox schemas. |
3 participants
Corrects the path definitions in the RADAR OpenAPI definition which duplicated the server path so the "Try It" function works correctly.
This PR fixes the RADAR OpenAPI path definitions so they align with the actual API routes and no longer duplicate the server prefix, which makes the “Try It” flow work correctly.
It also restructures the OpenAPI document to export
radarApiDoc, build schemas from shared TypeBox definitions, update metadata and security definitions, and reorganize endpoint paths for radars, controls, and targets to use relative route templates.