Skip to content

fix(parser): handle path-level parameters when filtering - #4450

Open
gary5 wants to merge 9 commits into
hey-api:mainfrom
gary5:fix/parser-path-level-parameters-filter
Open

gary5 wants to merge 9 commits into
hey-api:mainfrom
gary5:fix/parser-path-level-parameters-filter

Conversation

@gary5

@gary5 gary5 commented Sep 30, 2026

Copy link
Copy Markdown

Summary

Fix filtering for specs that use path-level parameters.

  • parser: include dependencies of #/paths/{path}/parameters in each operation of that path, so referenced component parameters/schemas are not dropped as orphans, and excluding them also excludes the affected operations.
  • parser: when filtering, remove path items with no HTTP methods left (previously only removed when the object was completely empty), so leftover path-level parameters no longer reference removed components. Applied to 2.0.x, 3.0.x and 3.1.x.

Tests added in filter.test.ts and the 3.0.x / 3.1.x parser tests.

Linked issues

Closes: #4449
Related to:

Certification

  • I have personally reviewed every line of this contribution and take responsibility for its correctness.

Path-level `parameters` on a path item (`#/paths/{path}/parameters`)
were never attributed as dependencies of the operations under that
path. With `parser.filters` enabled, this let dropOrphans delete
component parameters/schemas that were only referenced via path-level
$refs, and let operation-level include/exclude filters ignore that
usage entirely.
filterSpec dropped a path only when the whole path item object was
empty, so a path whose operations were all filtered out but which
still had a path-level `parameters` array survived filtering. The
parser then tried to resolve that array's $refs against components
already stripped by the filter, and threw.

Check for the presence of any HTTP method instead of the object being
completely empty.
@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@vercel

vercel Bot commented Sep 30, 2026

Copy link
Copy Markdown

@gary5 is attempting to deploy a commit to the Hey API Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6324e90

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hey-api/shared Patch
@hey-api/openapi-python Patch
@hey-api/openapi-ts Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pullfrog pullfrog Bot 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.

Important

The core fix is correct and well-targeted, but the new filterSpec deletion condition introduces regressions for path items that have no direct HTTP methods. See inline.

Reviewed changes

  • Path-level parameter dependencies attributed to operations — buildResourceMetadata collects the dependencies of #/paths/{path}/parameters and merges them into every operation under that path, so referenced components are no longer dropped as orphans and excluding a path-level parameter excludes its operations. The keying (path[1]) and lookup were verified correct for multi-segment and JSON-pointer-escaped paths, since operationPath === path[1] for operation nodes.
  • Path items with no operations removed — filterSpec replaces Object.keys(pathItem).length with httpMethods.some((method) => method in pathItem) across 2.0.x/3.0.x/3.1.x, so leftover path-level parameters no longer dangle after their referenced components are removed.
  • Tests — parser no-throw/deletion tests in 3.0.x and 3.1.x, plus two createFilteredDependencies unit tests.
  • Changeset — patch bump for @hey-api/shared.

ℹ️ webhooks and components.pathItems are not covered by the fix

buildResourceMetadata only inspects paths, components, and definitions, and filterSpec only iterates spec.paths. A 3.1 spec that declares path-item-level parameters under webhooks or components.pathItems hits the same orphan / dangling-$ref class of bug this PR fixes, and nothing here changes that. Worth confirming whether that is intentionally out of scope (to be tracked separately) rather than an oversight.

Technical details
# Path-level parameters outside `paths` are still unhandled

## Affected sites
- `packages/shared/src/openApi/shared/graph/meta.ts:87-96` — `pathParameterDependencies` is only built for `path[0] === 'paths'`.
- `packages/shared/src/openApi/3.1.x/parser/filter.ts:121-143` — `filterSpec` only iterates `spec.paths`; `webhooks`/`components.pathItems` are never filtered.
- `packages/shared/src/openApi/3.1.x/parser/index.ts:26` — `parseWebhooks` still parses these after filtering.

## Required outcome
- Decide and document whether path-item-level parameters under `webhooks` / `components.pathItems` should receive the same attribution treatment. If out of scope, a tracking issue is enough.

## Open questions for the human (optional)
- Is the omission intentional, or should `buildResourceMetadata`/`filterSpec` be made path-collection aware (e.g. `webhooks`, `components.pathItems`)?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment thread packages/shared/src/openApi/3.1.x/parser/filter.ts Outdated
Comment thread packages/shared/src/openApi/shared/graph/meta.ts Outdated
@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 41.90%. Comparing base (9d56b7e) to head (6324e90).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4450      +/-   ##
==========================================
+ Coverage   40.56%   41.90%   +1.33%     
==========================================
  Files         827      827              
  Lines       25727    25782      +55     
  Branches     7673     7707      +34     
==========================================
+ Hits        10436    10803     +367     
+ Misses      12319    12107     -212     
+ Partials     2972     2872     -100     
Flag Coverage Δ
unittests 41.90% <100.00%> (+1.33%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@hey-api/codegen-cli

npm i https://pkg.pr.new/@hey-api/codegen-cli@4450

@hey-api/codegen-core

npm i https://pkg.pr.new/@hey-api/codegen-core@4450

@hey-api/json-schema-ref-parser

npm i https://pkg.pr.new/@hey-api/json-schema-ref-parser@4450

@hey-api/nuxt

npm i https://pkg.pr.new/@hey-api/nuxt@4450

@hey-api/openapi-python

npm i https://pkg.pr.new/@hey-api/openapi-python@4450

@hey-api/openapi-ts

npm i https://pkg.pr.new/@hey-api/openapi-ts@4450

@hey-api/shared

npm i https://pkg.pr.new/@hey-api/shared@4450

@hey-api/spec-types

npm i https://pkg.pr.new/@hey-api/spec-types@4450

@hey-api/types

npm i https://pkg.pr.new/@hey-api/types@4450

@hey-api/vite-plugin

npm i https://pkg.pr.new/@hey-api/vite-plugin@4450

commit: 6324e90

gary5 and others added 6 commits September 30, 2026 17:21
…Metadata

The existing filter.test.ts cases hand-build a ResourceMetadata and
never call buildResourceMetadata itself, so a regression in the
path-level parameter attribution added in aaf487b would go
undetected. Exercise buildGraph + buildResourceMetadata directly
against a spec with a path-level `parameters` $ref instead.
…en filtering

filterSpec iterated every key of spec.paths without skipping `x-`
spec-extension entries, so a primitive extension value (e.g.
`{ 'x-foo': 'bar' }`) made `method in pathItem` throw. It also deleted
any path item left with no direct HTTP method keys, which incorrectly
dropped `$ref`-only path items (their operations live in the
referenced object, which filterSpec never resolves) instead of only
dropping items whose operations were actually filtered out.

Applied to 2.0.x, 3.0.x and 3.1.x, matching the `x-` skip already used
by each version's parser/index.ts.
buildResourceMetadata and filterSpec only ever looked at spec.paths,
so webhook operations were invisible to parser.filters entirely: with
orphans:false, a component only referenced by a webhook operation
looked unused and was dropped, and parseWebhooks then threw resolving
the now-missing $ref. Webhook-level `parameters` had the same
unattributed-dependency problem paths had before this PR.

Mirror the paths handling for webhooks (3.1.x-only, since 2.0.x/3.0.x
have no webhooks field):
- buildResourceMetadata collects `#/webhooks/{name}/{method}` operations
  and attributes `#/webhooks/{name}/parameters` dependencies to them.
- filterSpec (3.1.x) filters spec.webhooks the same way it filters
  spec.paths, skipping `x-` keys and preserving `$ref`-only webhooks.
- collectFiltersSetFromRegExpsOpenApiV3 matches operations.include/
  exclude regexps against webhook operations too.

Webhook operation keys reuse the existing `operation/` namespace
(webhook names can't start with `/`, so they can't collide with real
path keys), so no Filters config changes were needed.

components.pathItems is out of scope: it isn't parsed anywhere in this
codebase today, so wiring it into the filter would mean building new
parsing/IR support from scratch rather than extending an existing
filter path.
Codecov flagged the webhooks additions in the previous commit as
undercovered:
- shared/utils/filter.ts's new regex include/exclude branch for
  webhook operations in collectFiltersSetFromRegExpsOpenApiV3 was never
  exercised through the public createFilters() entry point.
- 3.1.x/parser/filter.ts's `x-` skip for spec-extension keys under
  `webhooks` (mirroring the existing paths one) had no covering case.

Verified via local coverage run that both files' new lines are now
fully hit; remaining gaps in those files are pre-existing and outside
this PR's diff.
A string extension value (the previous `'x-foo': 'bar'`) only
exercises the `x-` skip incidentally, since bracket-access property
reads on strings/numbers/booleans don't throw either way. `null` is
also a spec-legal extension value and does throw on property access,
so it actually requires the explicit `x-` skip rather than merely
tolerating a lucky operand type.
Replace the full pre-scan of every graph node with a direct pointer
lookup: the pointer to a path/webhook's `parameters` is fully derivable
from the operation's own path segment via pathToJsonPointer, so there's
no need to walk the entire graph just to find those nodes. Cache the
result per path/webhook so operations sharing a path (e.g. GET and
POST on the same route) don't recompute it per method.

Co-authored-by: Pintu Kumar <pksharmagh4@gmail.com>

This branch has not been deployed

No deployments
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.

Path-level parameters are dropped as orphans when filters are enabled

1 participant