feat(scm-multi-platform-detection): Implementing scoped matching#117720
Conversation
github-actions
Bot
added
the
Scope: Backend
| files_basenames = set(files_full_paths_by_basename.keys()) | ||
| dirs_basenames = set(dirs_full_paths_by_basename.keys()) |
There was a problem hiding this comment.
The some-only branch rebuilds both basename sets on every call, which the old tree_files = index.files materialization (removed here) was avoiding. Could we hoist them into detect_platforms_multi and pass them in so they're built once?
There was a problem hiding this comment.
Ah yep, adding this in now
There was a problem hiding this comment.
I added the change for this in the stacked branch already: #117745
…ed reads (#117745) Unified `_rule_parent_dirs` and _framework_matches_scoped to handle all rule types — existence, match_content, and match_package — in a single pass each. Both now accept content_by_path and manifests_by_path; passing empty dicts for these in the existence pass makes content/package rules silently return empty sets with no special-casing needed. Added content reads to detect_platforms_multi as a second high-confidence pass. After existence matching, `_collect_needed_paths` identifies candidate files, up to `MAX_CONTENT_READS` = 5 are fetched using the existing `_get_repo_file_content` and _parse_package_manifest helpers, sorted shallowest-first so root manifests are always within the cap. Supersession runs after both passes so content-detected frameworks can supersede existence matches. Added `TestCollectNeededPaths` and `TestDetectPlatformsMulti` covering the major branches: content-only detection, package-only detection, the shallow-first cap, content-driven supersession, and the zero-reads case.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5ab1180. Configure here.
|
|
||
| # every+some: at least one some rule must fire within a scope where all | ||
| # every rules are already satisfied. | ||
| return any(scope in scopes_for(rule) for scope in scopes for rule in some) |
There was a problem hiding this comment.
Android scope mismatch every some
Medium Severity
Scoped matching for frameworks with both every and some requires the same parent-directory scope for every rule and at least one some rule. For Android, match_dir app at the repo root yields scope "", while a typical app/build.gradle match yields scope app, so the framework never matches despite a valid Gradle Android layout.
Reviewed by Cursor Bugbot for commit 5ab1180. Configure here.
There was a problem hiding this comment.
Will take a look at this separately. Doesn't necessarily break all android detection, there's a test on a real repo
2 participants
Refactored
_TreeIndexand_build_tree_indexto track full paths separately for files (files_full_paths_by_basename) and directories (dirs_full_paths_by_basename), enabling location-aware detection across monorepo subtrees.Added
_parent_dirand_rule_parent_dirsto compute which parent directories a given rule is satisfiable in. Added_rule_existence_fires_in_scopeto check whether a rule fires within a specific directory scope. These feed into_framework_matches_scoped, which replaces the old flat matcher — for every rules it intersects per-rule parent dir sets, so a framework only fires when all its conditions are co-located in the same subtree. This prevents false positives from scattered files (e.g..csprojin one service andappsettings.jsonin another incorrectly matching dotnet-aspnetcore).Added unit tests in test_multi_platform_detection.py covering all four new/modified functions.