perf(integrate): narrow project-scope local discovery to .apm/ to avoid full-tree walks (closes #1507)#1554
Open
danielmeppiel wants to merge 1 commit into
Open
Conversation
…ithub/ (closes #1507) Local-scope integrate phase used to call discover_primitives(project_root) for the synthetic _local package, which walks the entire project tree via os.walk because LOCAL_PRIMITIVE_PATTERNS contains generic patterns like '**/*.instructions.md'. On a 50k+ file monorepo with only 2 instructions + 1 prompt under .apm/, this caused a 13-minute hang. Mirrors the existing $HOME narrowing (#830): when install_path == project_root (i.e. the synthetic _local PackageInfo built in services.integrate_local_content), restrict discover_primitives to .apm/ and .github/, which are the only supported locations for local primitives per LOCAL_PRIMITIVE_PATTERNS. The link_resolver.package_root sentinel for in-package asset rewriting (#1147) is preserved as the project root for the local case so asset links inside .apm/ are still resolved against the project tree. Tests: - Mock-based assertion that scan_root is .apm/ (and .github/ when present), never project_root. - Spy on os.walk to confirm noise subtrees are not traversed. - Mutation-break gate: removing the narrowing branch fails 4/4 new tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a performance bug (#1507) where apm install's integrate phase walks the entire project tree during local primitive discovery, causing multi-minute hangs in large monorepos. Extends the existing $HOME narrowing (#830) so that when install_path == project_root (the synthetic _local package), discovery is scoped to .apm/ and .github/ only.
Changes:
- Refactor
BaseIntegrator.init_link_resolverto support multiplescan_rootsand add a_is_root_local_packagehelper that narrows discovery for the project-scope_localpackage. - Preserve
link_resolver.package_root(asset rewriting from #1147) for installed deps and the project-scope local case; continue to skip it only for the$HOMEuser-scope case. - Add
TestInitLinkResolverLocalScoping(4 new tests, including a real-walk regression spy) and updatetest_uses_install_path_when_not_hometo reflect the real-package contract (apm_modules/owner/repo).
Show a summary per file
| File | Description |
|---|---|
| src/apm_cli/integration/base_integrator.py | Adds project-scope narrowing to .apm//.github/ and reworks the package_root assignment logic. |
| tests/unit/integration/test_base_integrator.py | Adds 4 unit tests covering narrowing, missing dirs, no-walk, and a real os.walk spy regression test. |
| CHANGELOG.md | Adds Unreleased Fixed entry referencing #1507. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 0
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.
Symptom
On a 50k+ file monorepo with only 2 instructions and 1 prompt under
.apm/,apm installhangs for ~13 minutes during the integrate phase before completing. Reported by @ioannispoulios in #1507.Trace
src/apm_cli/install/services.py:465-477builds a synthetic_localPackageInfowithinstall_path = project_rootand callsintegrate_package_primitives().src/apm_cli/integration/prompt_integrator.py:234invokesself.init_link_resolver(package_info, project_root).src/apm_cli/integration/base_integrator.py:531-553(pre-fix) setsscan_root = package_info.install_pathand only narrows toscan_root/.apmwhenscan_root == Path.home()(issue User-scope install (--global) unexpectedly enters local.apmintegration and can spend a long time scanning$HOME#830). For project-scope local content,scan_rootstays at the project root.src/apm_cli/primitives/discovery.py:590walksbase_dirwithos.walk. BecauseLOCAL_PRIMITIVE_PATTERNScontains generic patterns like**/*.instructions.md, the walk traverses the entire repo even when no local primitive lives outside.apm/. Result: O(repo size) work proportional to the monorepo, not to APM content.Fix
Extend the existing
$HOMEnarrowing to also apply wheninstall_path == project_root(i.e. the synthetic_localPackageInfo). For that case, scan only.apm/and.github/-- the two supported locations for local primitives perLOCAL_PRIMITIVE_PATTERNS. Real installed packages (apm_modules/<owner>/<repo>/...) keep scanning theirinstall_pathdirectly because theirinstall_pathdiffers fromproject_root.The
link_resolver.package_rootsentinel for in-package asset rewriting (#1147) is preserved as the project root for the local case, so asset links inside.apm/continue to resolve against the project tree.Tests (TDD + mutation-break gate)
tests/unit/integration/test_base_integrator.py::TestInitLinkResolverLocalScoping:test_narrows_to_apm_and_github_when_install_path_is_project_root-- mocksdiscover_primitives, asserts it is called with.apm/and.github/and never withproject_rootitself.test_skips_missing_directories-- only.apm/exists, so only.apm/is scanned.test_no_apm_or_github_means_no_walk-- no scannable dirs,discover_primitivesis never called.test_real_walk_does_not_traverse_noise_subtree-- end-to-end with a real walk; spies onos.walkto assert nonoise/...directory is visited even though the noise subtree contains a file matching the generic**/*.instructions.mdpattern.Mutation-break gate: deleting the narrowing branch causes all four new tests to fail.
The pre-existing
test_uses_install_path_when_not_homewas updated to useinstall_path = tmp_path / "apm_modules" / "owner" / "repo"so it reflects the real-package contract (install_path != project_root) it was always meant to exercise.How to test on a synthetic large-tree fixture
Before this PR: minutes. After: sub-second integrate phase.
Validation evidence
uv run --extra dev pytest tests/unit/integration/ tests/unit/install/ tests/unit/test_local_content_install.py -q-> 2959 passed.ruff check,ruff format --check,pylint R0801,lint-auth-signals.sh) -> all silent / 10.00.Closes #1507.