Repository navigation
Conversation
|
Drive-by note from #1507, in case it helps this get looked at: this PR also fixes a bug nobody has connected to it. #1507 (opened today) reports I ran both predicates against the same inputs at
The JVM/Swift CamelCase shapes ( The one gap: your two new tests are both Python, so nothing here would catch a Go regression. If you'd like, I can send a commit adding |
|
Thanks for the careful validation and for connecting this to #1507. Please go ahead and add math.go / math_test.go regression coverage to this branch — I agree it should be included before merge. |
|
Hi @maxmilian, thanks again for offering to add the Go regression coverage. Just checking in — are you still planning to send the follow-up commit for math.go / math_test.go? No worries if timing is tight; I’m happy to add it on my side instead. Please let me know if you need anything from me to help move this forward. |
|
Hi @colbymchenry, I noticed #1507 was closed via #1803, based on #1688. My PR #1273 has been open since July 13 and already proposed replacing the duplicated test-file detection. On August 5, @maxmilian verified that it also fixes #1507 and linked it in both discussions. Could you review this earlier contribution and clarify how it should be acknowledged? My PR also includes a separate Thanks. |
Jseca666
changed the title
…oundary `codegraph affected --filter` turned its glob into an unanchored regex by hand (`**` to `.+`, `*` to `[^/]*`, no `^` or `$`), so it matched anywhere inside a path: `tests/**/*.py` missed `tests/x.py` (`.+` needs a folder), a changed `tests/x.py.bak` counted as a test under `tests/*.py`, the changed test itself was missed under `tests/**/*.py`, `e2e/*.spec.ts` took an `xe2e/` folder, and `/tests/*.py` matched only `other/tests/...`. The filter is now a picomatch glob (an existing dependency, loaded only by this command) with `dot: true`, matched against the whole project-relative path. A pattern with no slash matches the file name at any depth. One with a slash matches from any folder boundary, an implicit leading `**/`, which keeps the help text's own `e2e/*.spec.ts` matching `apps/web/e2e/...` as the filter always did; a leading `/` or `./` anchors it at the project root, and a trailing `/` takes everything under that folder. A pattern picomatch rejects is reported as an invalid --filter instead of failing later. Validation: new CLI-level cli-affected-filter-globs tests (6): 4 fail on main's source and pass with the fix; the other 2 guard behavior kept on purpose (file-name patterns at any depth, folders outside the test conventions, dotfiles, a trailing-slash folder). The existing affected CLI suites pass. The triage fixture shows the same before and after. Reimplements PR #1273 by @Jseca666. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…oundary (#3029) `codegraph affected --filter` turned its glob into an unanchored regex by hand (`**` to `.+`, `*` to `[^/]*`, no `^` or `$`), so it matched anywhere inside a path: `tests/**/*.py` missed `tests/x.py` (`.+` needs a folder), a changed `tests/x.py.bak` counted as a test under `tests/*.py`, the changed test itself was missed under `tests/**/*.py`, `e2e/*.spec.ts` took an `xe2e/` folder, and `/tests/*.py` matched only `other/tests/...`. The filter is now a picomatch glob (an existing dependency, loaded only by this command) with `dot: true`, matched against the whole project-relative path. A pattern with no slash matches the file name at any depth. One with a slash matches from any folder boundary, an implicit leading `**/`, which keeps the help text's own `e2e/*.spec.ts` matching `apps/web/e2e/...` as the filter always did; a leading `/` or `./` anchors it at the project root, and a trailing `/` takes everything under that folder. A pattern picomatch rejects is reported as an invalid --filter instead of failing later. Validation: new CLI-level cli-affected-filter-globs tests (6): 4 fail on main's source and pass with the fix; the other 2 guard behavior kept on purpose (file-name patterns at any depth, folders outside the test conventions, dotfiles, a trailing-slash folder). The existing affected CLI suites pass. The triage fixture shows the same before and after. Reimplements PR #1273 by @Jseca666. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @Jseca666 for finding this! We landed our own implementation in #3029: |
…er globs (#589) * fix(spring): a route links the method it annotates, including a package-private one (colbymchenry#3009) The Spring extractor found a mapping's handler with a pattern that wanted `public`, `private` or `protected` (or Kotlin's `fun`) before the name, in the next 600 characters. A package-private handler, like Spring Boot 3's `@GetMapping List<Article> list()`, matched nothing, so its route linked the next method declared with a modifier, or nothing at all, and a method-level `@RequestMapping(value = "/feed", method = GET)` on such a method made no route. A generic Kotlin `fun <T> find()` handler fell through the same way. Separately, a route bound by name alone took the wrong overload when a tall annotation stack (`@Operation`, `@ApiResponses`) above the mapping left the next overload's declaration nearer the annotation. The extractor now reads the declaration the annotation is written on: past it and every annotation stacked with it (arguments read as balanced groups, strings and text blocks included), the modifiers (none at all for a package-private method), type parameters and the return type (void, a primitive, a qualified or nested generic type, an array) to `name(`; for Kotlin, past its modifiers to `fun`, type parameters and `name(`. A field, a constructor or a class under the annotation gives no handler. The mapping annotation's own arguments are read the same balanced way. At resolution, the route's handler reference binds to the method of that name whose span (which starts at its first annotation) holds the annotation's line, the innermost such method; any other case resolves by name as before. Validation, kernel engine, main 01e0ac3 vs this commit: - realworld-java21 93e018e: Spring routes linked 4/19 -> 19/19; the 15 new edges each go to the method the mapping is written on. - spring-petclinic 500158f (17 routes), spring-petclinic-kotlin 817dba3 (18), eladmin 55fbf70 (133), mall dcaa93b (246), dolphinscheduler 21968bc (239): same targets, only the edge metadata changes (exact-match 0.7/0.9 -> framework 0.95). - Controls jsoup 0c508f1 (Java, no Spring), requests 3c7baaa (Python), cobra adbc881 (Go): dump-graph byte-identical. - Index time, median of 3 interleaved: mall 12.3 s vs 12.6 s, eladmin 3.02 s vs 3.03 s (noise on a shared machine). - New tests fail on main's java.ts (20 failing) and pass with the fix, in both engines; dropping the positional binding fails only the overload test. Reimplements PR colbymchenry#851 by @Liurchen, who found the package-private gap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 3d19da2) * fix(affected): a --filter glob matches whole names, from any folder boundary (colbymchenry#3029) `codegraph affected --filter` turned its glob into an unanchored regex by hand (`**` to `.+`, `*` to `[^/]*`, no `^` or `$`), so it matched anywhere inside a path: `tests/**/*.py` missed `tests/x.py` (`.+` needs a folder), a changed `tests/x.py.bak` counted as a test under `tests/*.py`, the changed test itself was missed under `tests/**/*.py`, `e2e/*.spec.ts` took an `xe2e/` folder, and `/tests/*.py` matched only `other/tests/...`. The filter is now a picomatch glob (an existing dependency, loaded only by this command) with `dot: true`, matched against the whole project-relative path. A pattern with no slash matches the file name at any depth. One with a slash matches from any folder boundary, an implicit leading `**/`, which keeps the help text's own `e2e/*.spec.ts` matching `apps/web/e2e/...` as the filter always did; a leading `/` or `./` anchors it at the project root, and a trailing `/` takes everything under that folder. A pattern picomatch rejects is reported as an invalid --filter instead of failing later. Validation: new CLI-level cli-affected-filter-globs tests (6): 4 fail on main's source and pass with the fix; the other 2 guard behavior kept on purpose (file-name patterns at any depth, folders outside the test conventions, dotfiles, a trailing-slash folder). The existing affected CLI suites pass. The triage fixture shows the same before and after. Reimplements PR colbymchenry#1273 by @Jseca666. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 60b0d7a) * docs(readme): track upstream through 60b0d7a * fix(spring): read type-use annotations inside return types (colbymchenry#3009 review) `annotatedMethodName` stopped at a type-use annotation in a qualified or array return type, so `public String @nullable [] names()` and `public java.util.@nullable List<String> names()` lost their route's handler link. Read annotations at each return-type position and add both declarations, plus a method-level `@RequestMapping`, to the Spring test. Also correct two copied [Unreleased] bullets that claimed the CLI read commands open the index read-only and report a rebuild: the fork's CLI still opens read-write, so the claims did not describe this fork. Document the affected filter's whole-name, folder-boundary, root-anchor and trailing-slash matching in the README and the site guide. * fix(spring): span consecutive type-use annotations after a qualifier `java.util.@nullable @readonly List<String>` left the second annotation unread, so the method name lookup hit `@` and the route lost its handler. Consume every consecutive annotation after the dot, cover both a `@GetMapping` and a method-level `@RequestMapping` handler, and add the missing "no slash matches the file name" rule to the README's affected filter description. --------- Co-authored-by: Colby Mchenry <me@colbymchenry.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
codegraph affected --filter 'tests/**/*.py'misses tests directly insidetests/, while--filter 'tests/*.py'can also selectother/tests/...or a changed.py.bakfile. This change uses the existingpicomatchdependency to match glob patterns correctly.Filters containing
/match the whole project-relative path, and**/matches zero or more directories. Filename-only filters such as*_test.gocontinue to match at any depth, and explicit custom filtering still includes dotfiles.This PR now focuses on the remaining custom-filter fix from #1273. The default test-file recognition is already covered by #1803; its shared
isTestPathclassifier and existing Go/Python/JVM regression tests are retained. The branch is synchronized withmainand preserves the original contribution history.Validation
npm run buildpassed on Windows with Node 24.14.1.cli-affected-filter,cli-affected-paths,cli-affected-test-conventions,is-test-file, andgraph.git diff --checkpassed.npm auditreports 10 existing dependency advisories (1 critical, 4 high, 5 moderate). Dependency manifests and lockfiles match upstream; this PR adds or updates no dependencies.