Skip to content

fix(swagger): correct malformed path annotations in experiment and flag controllers - #3405

Merged
bcb37 merged 1 commit into
devfrom
fix/swagger-path-annotations
Oct 28, 2026
Merged

bcb37 merged 1 commit into
devfrom
fix/swagger-path-annotations

Conversation

@bcb37

@bcb37 bcb37 commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Ten @swagger annotations didn't describe the routes they document, so the generated spec advertises paths and payloads that don't exist. Comment-only — no routing or behaviour changes.

Found while regenerating the spec for the GitBook API reference: GitBook rejected the upload with processing errors, all of which traced back to these.

1. Literal path segments wrapped in braces

swagger-jsdoc reads {validation} as a path parameter named validation, which is then never declared:

Was Now Actual route
/experiments/{validation} /experiments/validation @Post('/validation')
/experiments/{import} /experiments/import @Post('/import')
/experiments/{export} /experiments/export @Get('/export')

2. Express-style parameter instead of Swagger templating

PUT /experiments/:id → /experiments/{id}. The DELETE on the same route was already annotated correctly, so the spec listed the two as separate paths. They now share one entry with both methods.

3. Path parameters declared with the wrong key

Five operations used id: or — copy-pasted out of the flag controller — flagId: where OpenAPI expects name:. The parameter was nameless and matched nothing in the path template:

  • /experiments/export/includeLists/{id}
  • /experiments/export/excludeLists/{id}
  • /flags/export/{id}
  • /flags/export/includeLists/{id}
  • /flags/export/excludeLists/{id}

All five handlers bind :id, so name: id is correct for each.

4. GET /experiments/export had the import annotation copied onto it wholesale

It documented a required request body of experiment files, but the handler is @QueryParams() params: ExperimentIds and reads params.ids. Its 200 also claimed an array of {fileName, error} when it returns ExperimentDTO[].

Replaced with an in: query array of uuids (matching ExperimentIds: @IsArray @IsNotEmpty @IsUUID('all', {each: true}) ids: string[]) and a response referencing ExperimentResponse, the idiom GET /experiments/single/{id} already uses.

Verification

Regenerated the spec with the same swagger-jsdoc options as swaggerLoader.ts, before and after:

  • Malformed paths: 9 → 0
  • Operation count: 111 → 111 (unchanged — nothing gained or lost)
  • Path count: 94 → 93 (PUT/DELETE now share /experiments/{id})

Confirmed the two POST body annotations on /experiments/validation and /experiments/import are correct as-is — in: body is right for a POST, and their schema matches ExperimentFile { fileName, fileContent }.

Not included

ExperimentClientController.v6.ts:1039 annotates DELETE /clearDB, but the handler sits under @JsonController('/v6/'), so the real route is /v6/clearDB. Left out deliberately — fixing it also means re-publishing the GitBook spec and updating the operation reference on the API reference page, so it's better handled alongside those.

🤖 Generated with Claude Code

…ag controllers

Ten @Swagger annotations did not describe the routes they document, so the
generated spec advertised paths and payloads that do not exist.

Literal path segments were wrapped in braces, making swagger-jsdoc read them
as path parameters that were then never declared:
  /experiments/{validation} -> /experiments/validation
  /experiments/{import}     -> /experiments/import
  /experiments/{export}     -> /experiments/export

PUT /experiments used an Express-style parameter rather than Swagger
templating, so it appeared as a path of its own instead of joining the
DELETE on the same route:
  /experiments/:id -> /experiments/{id}

Five path parameters were declared with the wrong key — `id:` or, from a
copy-paste out of the flag controller, `flagId:` — where OpenAPI expects
`name:`. The parameter was therefore nameless and matched nothing in the
path template:
  /experiments/export/includeLists/{id}
  /experiments/export/excludeLists/{id}
  /flags/export/{id}
  /flags/export/includeLists/{id}
  /flags/export/excludeLists/{id}

GET /experiments/export had the import endpoint's annotation copied onto it
wholesale: it documented a required request body of experiment files, when
the handler takes @queryParams() and reads params.ids, and its 200 response
claimed an array of {fileName, error} when it returns ExperimentDTO[].
Replaced with an in: query array of uuids and a response referencing
ExperimentResponse, matching the idiom used by GET /experiments/single/{id}.

Comment-only; no routing or behaviour changes. Operation count is unchanged
at 111, and path count drops 94 -> 93 because PUT and DELETE now share
/experiments/{id}. Regenerating the spec reports 0 malformed paths, down
from 9.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 27, 2026 20:04
@bcb37
bcb37 requested review from danoswaltCL and zackcl October 27, 2026 20:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

There are no active review issues for the Swagger annotation updates.

No open issues 🎉

What changed in this PR

This comment-only PR aligns the backend’s Swagger annotations with existing experiment and feature flag endpoints, correcting generated API documentation without changing routing or runtime behavior.

Changes:

  • Corrects literal route segments and Swagger path-parameter syntax.
  • Replaces malformed parameter keys with name: id on five export operations.
  • Documents experiment export with an ids query array and an ExperimentResponse array response.
File Description
packages/​backend/​src/​api/​controllers/​FeatureFlagController.ts Corrects path-parameter names in three export annotations.
packages/​backend/​src/​api/​controllers/​ExperimentController.ts Corrects documented paths, export parameters, and the export response schema.

Review effort: Balanced

@bcb37
bcb37 merged commit 8255c02 into dev Oct 28, 2026
7 checks passed
@bcb37
bcb37 deleted the fix/swagger-path-annotations branch October 28, 2026 14:36
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.

3 participants