Repository navigation
Conversation
 [Medium impact] The change reaches code generation runs that use operation filters. Preserve compatibility for previously generated types before merging. FindingsFix with agent prompt### Issue 1
pkg/codegen/filter.go:40-41
Deleting the whole path also removes references from its shared parameters. If a filtered-out path has a parameter referencing `#/components/schemas/ItemID`, that reference previously kept `ItemID` in the generated Go code. Now `pruneUnusedComponents` deletes it when nothing else references it, breaking downstream code that uses the exported type. Both filter helpers have this problem.
The repository requires compatibility protection for changes to generated APIs. Preserve the existing type output by default or make this broader change opt-in.
### Issue 2
pkg/codegen/filter_test.go:161-170
The new test checks only paths with one operation each. Add a path that starts empty and a path with both a kept and a removed operation, then assert that both paths survive under each filter kind. These cases protect the two deletion guards; the current tests would still pass if either guard were removed.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThe PR removes paths after tag or operation-ID filtering removes their last operation.
Reviews (1) · Last reviewed commit: "Drop paths with no operations left after..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if len(names) > 0 && len(pathItem.Operations()) == 0 { | ||
| paths.Delete(path) |
There was a problem hiding this comment.
Deleting the whole path also removes references from its shared parameters. If a filtered-out path has a parameter referencing #/components/schemas/ItemID, that reference previously kept ItemID in the generated Go code. Now pruneUnusedComponents deletes it when nothing else references it, breaking downstream code that uses the exported type. Both filter helpers have this problem.
The repository requires compatibility protection for changes to generated APIs. Preserve the existing type output by default or make this broader change opt-in.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/filter.go
Line: 40-41
Comment:
**Previously generated types disappear**
Deleting the whole path also removes references from its shared parameters. If a filtered-out path has a parameter referencing `#/components/schemas/ItemID`, that reference previously kept `ItemID` in the generated Go code. Now `pruneUnusedComponents` deletes it when nothing else references it, breaking downstream code that uses the exported type. Both filter helpers have this problem.
The repository requires compatibility protection for changes to generated APIs. Preserve the existing type output by default or make this broader change opt-in.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| t.Run("by tag", func(t *testing.T) { | ||
| swagger := load(t) | ||
| filterOperationsByTag(swagger, Configuration{OutputOptions: OutputOptions{IncludeTags: []string{"kept"}}}) | ||
| assert.Equal(t, []string{"/kept"}, swagger.Paths.Keys()) | ||
| }) | ||
|
|
||
| t.Run("by operation id", func(t *testing.T) { | ||
| swagger := load(t) | ||
| filterOperationsByOperationID(swagger, Configuration{OutputOptions: OutputOptions{ExcludeOperationIDs: []string{"getDropped"}}}) | ||
| assert.Equal(t, []string{"/kept"}, swagger.Paths.Keys()) |
There was a problem hiding this comment.
The new test checks only paths with one operation each. Add a path that starts empty and a path with both a kept and a removed operation, then assert that both paths survive under each filter kind. These cases protect the two deletion guards; the current tests would still pass if either guard were removed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/filter_test.go
Line: 161-170
Comment:
**Path preservation lacks tests**
The new test checks only paths with one operation each. Add a path that starts empty and a path with both a kept and a removed operation, then assert that both paths survive under each filter kind. These cases protect the two deletion guards; the current tests would still pass if either guard were removed.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
When include-tags, exclude-tags, include-operation-ids or exclude-operation-ids removed every operation of a path, the path item stayed in the spec as an empty entry, so the embedded spec still listed it (for example
"/t2":{}). I now delete a path once filtering has removed all of its operations. Paths that never had operations are left alone. Two existing prune tests asserted the old behaviour and now check that the path is gone, and I added a test covering both filter kinds.