fix: name a route after its own decorator, not the file's first - #61
Merged
Merged
Conversation
The Python adapter looked a route's path up with `file.calls.find` by the decorator's dotted name, so every `@app.post` in a module resolved to the first one. A real Flask module with 57 routes produced 55 nodes carrying 4 distinct names, and `documentedCapabilityLabel` then matched 18 unrelated handlers to one capability. One route per file hid it: the file-wide lookup and the per-declaration lookup return the same call. The extractor already records each decorator call inside the scope of the function it decorates, on a line above the `def`, so a route now reads the nearest such call above its own declaration. `enclosingClass` is not part of the match: a route declared inside a method carries the class on the call but not on the declaration, so comparing the two loses the path instead of sharpening it, while the nearest-above rule already separates same-named methods in different classes. The existing fixtures could not catch this, so the two new tests cover two same-method routes in one module and the scope cases that pin the tie-break. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #60.
What was wrong
routeFromDecoratorsresolved a route's path withfile.calls.find((item) => item.callee === decorator && item.stringArguments.length).file.callsis the flat call list for the whole file, so every@app.postin a module resolved to the firstapp.post(...)in that file.Reproduced before touching anything, with the module from #60:
file:line, symbol and edges were all correct — onlynameandmetadata.pathwere wrong, which is enough to makedocumentedCapabilityLabelmatch on the wrong path and collapse a single-module app's feature list.One route per file hid it: the file-wide lookup and the per-declaration lookup return the same call, and no fixture put two same-method routes in one module.
The fix
extract.pyalready emits everything needed — it visitsdecorator_listinside the function scope, so each decorator call carriesenclosingFunctionset to the decorated function and a line just above thedef. This is a TypeScript-side change only.A route now reads the nearest matching call above its own declaration:
Why
enclosingClassis not comparedThe patch suggested in #60 also required
item.enclosingClass === declaration.enclosingClass. That breaks a route declared inside a class method: the extractor records the class on the call but not on the nested declaration, so the two never match and the path degrades to/.The nearest-above rule already separates same-named methods in different classes, so the class comparison only loses information.
Evidence
Red before green. The two new tests were written first and run against the unmodified adapter: exactly those 2 failed, the existing 7 passed. After the fix: 9 passed.
Mutation proof. Each plausible wrong implementation was applied in turn and had to turn a test red (implementation restored byte-identical afterwards):
enclosingClassto matchThe first draft of the tests did not catch D — the nested case was inside a plain function rather than a class method. The
class Wiringcase was added for it.Full
npm run check: typecheck passes, 24 files / 213 tests pass (211 before, 2 new), build passes.TypeScript adapter checked too: it reads the path off the call expression it is iterating (
adapters/typescript/src/index.ts:911), so it resolves per call site and does not share this bug. No change there.🤖 Generated with Claude Code