refactor: tighten Stage 2 complexity thresholds (#1078)#1681
Open
sergio-sisternes-epam wants to merge 20 commits into
Open
refactor: tighten Stage 2 complexity thresholds (#1078)#1681sergio-sisternes-epam wants to merge 20 commits into
sergio-sisternes-epam wants to merge 20 commits into
Conversation
Split commands/install.py (2085 -> 765 lines) into six install/ submodules: pkg_resolution, apm_packages, install_cmd_phases, mcp_handler, manifest_rollback, cli_context. Reduces statements/branches/complexity and file-length below the issue #1078 Stage 2 targets while preserving the public command surface and @patch test seams via module-level re-exports. - RULE A: moved helpers re-exported from commands.install so @patch('apm_cli.commands.install.*') targets keep resolving. - RULE B: moved call sites route patched symbols through the commands.install module object (_m.) so monkeypatches intercept across module boundaries. - Removed the duplicated inline local-bundle block; wired the extracted _try_local_bundle_install helper (R0801 duplication guardrail green). Behaviour-preserving: 16645 unit + acceptance and 927 install integration tests pass; ruff (std + final thresholds), ruff format, and R0801 all green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#1078) Bring install/services.py under Stage 2 thresholds and <800 lines (859->684) by reusing existing structure instead of moving code: - integrate_local_content: collapse six separate integrator params into the existing IntegratorBundle (15->10 args). - integrate_package_primitives: group optional knobs (skill_subset, scratch_root, policy) into a new frozen IntegrationOptions dataclass (14->12 args); extract per-kind/skill logging + cowork-warn helpers into new sibling install/services_integrate.py to clear C901/branch/stmt. - integrate_local_bundle: extract slug validation helper to clear C901. Monkeypatch seams preserved (both functions stay defined in services.py); public names and backward-compat aliases re-exported. Behaviour unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… + LSPIntegrationContext (#1078) Introduce MCPRequestSpec frozen dataclass (mcp/spec.py) grouping the six shared MCP identity fields; run_mcp_install drops 15->10 args and validate_mcp_conflicts 14->9 args by consuming it. Add LSPIntegrationContext to lsp/integration.py so run_lsp_integration drops 15->8 args. Call sites in mcp_handler.py, commands/install.py, apm_packages.py updated; unit/integration call sites updated to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ownloader Move the ~285-line nested download_callback closure out of _resolve_dependencies into a stateful _TransitiveDownloader callable in a new sibling module install/phases/resolve_transitive.py. Ruff folds nested closures into the parent function, so this clears the C901/PLR0915 hot spot on _resolve_dependencies and drops resolve.py from 988 to 691 lines (under the 800 file-length target); the new module is 344 lines. The callable keeps parent_pkg in __call__ so APMDependencyResolver's _signature_accepts_parent_pkg introspection still routes the declaring-parent anchor for transitive local deps (#857). The registry branch stays inline in __call__ so the dep_ref rebind remains visible to the shared except handler; this is safe because get_unique_key() is invariant to registry_name. The download accumulators (downloaded/failures/transitive_failures) are read back onto ctx by identity, preserving post-resolution behaviour exactly. Refs #1078 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stage 2 file-length tightening for the install dependency-source
strategy. sources.py 886 -> 519 lines (<800) with no behavioural change.
- New sources_base.py: Materialization value object + DependencySource
ABC, now with two hoisted helpers that fold the per-source lockfile
bookkeeping that was copy-pasted into every acquire():
* _lockfile_node_fields() -> (depth, resolved_by, is_dev)
* _skip_integration(deltas) -> Materialization(package_info=None, ...)
- New sources_fresh.py: FreshDependencySource (network path) plus
_format_package_type_label, moved verbatim.
- sources.py keeps Local + Cached + _rebuild_cached_semver_resolution +
make_dependency_source, re-exports the moved symbols (with __all__) so
every existing import path keeps working, and uses the new base helpers
in Local/Cached.
Less code, same functionality: the three identical node-field blocks and
two identical skip-integration returns collapse into shared base helpers.
R0801 green.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stage 2 complexity tightening for the install targets phase. targets.py run() C901 45 -> ~8, PLR0915 143 -> ~35, PLR0912 50 -> ~7, all well under the Stage 2 thresholds (35/120/40). File 576 -> 585. Extracted five focused module-level helpers from run(): - _gate_cowork_target / _gate_copilot_app_target (kept separate: cowork has Linux-vs-other messaging + a project-scope gate copilot-app lacks) - _resolve_project_scope_targets (v2 resolution block) - _handle_user_scope_targets (user-scope logging + dir creation) - _build_integrators (integrators dict) run() is now a thin orchestrator. No behavioural change: call sites, argument order, log messages, exit codes, and exception types preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n helpers Move `_preflight_auth_check` into a new sibling `pipeline_preflight.py` (re-exported for import compatibility) and extract three cohesive helpers from `run_install_pipeline` -- `_read_early_lockfile_state`, `_resolve_managed_files`, and `_run_skill_path_migration`. This drops `run_install_pipeline` under the Stage 2 complexity thresholds (statements/branches/mccabe) and pipeline.py under 800 lines, with no behaviour change. `_run_phase` and `run_install_pipeline` remain module-level (test monkeypatch surface preserved). Refs #1078 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ge modules Extract the pure stateless hook transforms into `hook_transforms.py` and the source-marker / dependency-scan / merge helpers into `hook_merge.py`, and decompose `_integrate_merged_hooks` into a thin orchestrator over focused helpers. A shared `_copy_hook_scripts` removes a duplicated script-copy loop. hook_integrator.py drops 1738 -> 769 lines and `_integrate_merged_hooks` falls under the Stage 2 complexity thresholds (mccabe/branches/statements), with no behaviour change. Module-level names tests patch/import (`HookIntegrator`, `_rich_warning` via wrapper, the re-exported transforms) are preserved. Refs #1078 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…olds Decompose skill_integrator.py (1827 lines) into cohesive siblings: skill_naming, skill_orchestrate, skill_plugin, skill_sync, skill_deploy. skill_integrator now layers over skill_deploy; get_effective_type and should_install_skill remain module-level to preserve monkeypatch targets. validate_skill_name return count reduced under the Stage 2 max-returns gate. Part of #1078 (Strangler Stage 2). Behaviour-preserving; full unit, acceptance and integration suites green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n gate Move the TOCTOU-safe _read_bytes_no_follow + _SymlinkRaceError into a cohesive base_integrator_io sibling (re-exported, so the original import/patch path is preserved). Collapse validate_deploy_path's tail to a single return (PLR0911 9->8) and reuse the existing partition_bucket_key helper in partition_managed_files instead of inlining the alias lookup twice. base_integrator.py 812->769 lines (under the 800 gate). Part of #1078 (Strangler Stage 2). Behaviour-preserving; full suites green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move RULE_FORMATS, PrimitiveMapping and TargetProfile out of targets.py into a focused target_profile module; targets.py re-exports all three so the 178x-imported public surface (and KNOWN_TARGETS, which stays put) is unchanged. No behaviour change; the dataclasses are self-contained. targets.py 1036->631 lines (under the 800 gate). Part of #1078 (Strangler Stage 2). Full unit, acceptance and integration suites green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Decompose _resolve_target_runtimes (C901 50, PLR0912 54, PLR0915 132) into in-module patch-safe helpers: _detect_installed_runtimes (+ fallback and a data-driven _runtime_opted_in predicate that collapses the parallel cursor/opencode/gemini/windsurf opt-in branches via _DIR_GATED_RUNTIMES), _intersect_script_runtimes, and _apply_user_scope_filter. find_runtime_binary stays a module global so the 38 monkeypatch sites remain valid. Fix _install_registry_group PLR0913 (13 args) with a cohesive _RegistryDepGroup parameter object (deps/names/dep_map), keeping the callable behaviour stable. All six Stage-2 thresholds now clean for this file; 785 lines (<800). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract the JSON/TOML/Claude config-cleanup trio into a new mcp_config_clean.py, and collect_transitive / _install_for_runtime / _gate_project_scoped_runtimes bodies into a new mcp_runtime_ops.py. MCPIntegrator keeps thin delegating staticmethods so the heavily monkeypatched MCPIntegrator.<name> call/patch surface is unchanged; patched module globals (_rich_success, Path) are routed back through mcp_integrator from the siblings so the patch targets stay honored. mcp_integrator.py 1150 -> 765 lines (file-length offender cleared). File-length backlog 30 -> 29. Issue: #1078 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-line budget Strangler Stage 2 (#1078), Commit 3a: tighten the deps subsystem's lower-risk files toward the 800-line guardrail and the final complexity thresholds, preserving the test monkeypatch surface. File-length offenders cleared (siblings extracted, delegating wrappers keep patch targets intact): - apm_resolver.py 1028 -> 777 (new apm_resolver_helpers.py) - plugin_parser.py 916 -> 632 (new plugin_server_helpers.py) - bare_cache.py 805 -> 724 (new bare_cache_msg.py) Complexity-only (PLR0911) fixes, behaviour-preserving: - git_reference_resolver.py: extract _call_commits_api; merge guard - lockfile.py: combine two equality guards into one return - registry/outdated.py: merge early-exit guards, preserve source strings bare_cache failure-message builder uses a genuine parameter object: build_clone_failure_message now takes a required CloneFailureContext (bundling the six failure-classifier flags) instead of flat kwargs; clone_engine.py (sole caller) constructs it at the call site. No **kwargs threshold-gaming; 13 call args -> 8 params. backlog 29 -> 26; R0801 10.00/10; 16645 unit+acceptance pass; 2776 targeted integration pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-line budget Strangler Stage 2 (#1078), Commit 3b: bring the two largest deps files under the 800-line guardrail and the final complexity thresholds while preserving the heavy test monkeypatch surface. File-length offenders cleared via delegating wrappers + ops siblings (every new module is <=525 lines, well under budget): - github_downloader.py 1806 -> 737; bodies moved to github_downloader_setup_ops.py (247), github_downloader_package_ops.py (525), github_downloader_subdir_ops.py (448) - download_strategies.py 1123 -> 587; bodies moved to download_strategies_ops.py (379), download_strategies_backends_ops.py (416); DownloadDelegate class stays in download_strategies.py Complexity offenders decomposed in their new homes (genuine cohesive helpers, distinct paths kept separate -- NOT collapsed): - download_subdirectory_package C901/PLR0912/PLR0915 -> per-cache-tier helpers (persistent / shared-bare / legacy-clone) + extract/build/log - download_github_file C901/PLR0912 -> per-step + per-backend helpers Patch-safety: thin class wrappers keep patched method names; moved bodies route patched module globals via function-level _gh./_ds. aliases (Rule B); inter-ops calls route through the class wrappers so no ops module imports another at module scope (no cycles). No **kwargs gaming, no noqa, no threshold edits. backlog 26 -> 24; R0801 10.00/10; 16645 unit+acceptance pass; 3030 targeted integration pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…1078) Strangler Stage 2, Commit 4 of 8. Drive the commands and bundle subsystems under the 800-line file guardrail and clear their second-tier complexity offenders, preserving behaviour and the monkeypatch/import surface via re-export shims and delegating wrappers. Length splits (all resulting modules < 800): - marketplace/__init__.py 1601 -> 535 (+_table_ops 350, _registry_cmds 543, _publish_ops 261, _search_cmd 119) - commands/audit.py 1161 -> 758 (+_audit_ops 372) - commands/deps/cli.py 925 -> 673 (+_cli_ops 273) - commands/pack.py 808 -> 694 (+_pack_ops 168) - compile/cli.py 1002 -> 671 (+_run_ops 417) Complexity-only fixes (genuine simplification, no suppression): - compile _run_compilation: 13 args -> 5 via frozen CompilationRunConfig parameter object constructed at the call site - marketplace doctor run_doctor: extracted 8 cohesive check helpers - uninstall, outdated, find, plugin_exporter, local_bundle: merged early-return guards and extracted cohesive helpers to clear PLR0911/PLR0912/PLR0915/C901 File-length backlog 24 -> 19. Complexity gate clean at final Stage-2 thresholds; thresholds themselves flip in the final enforcement commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Strangler Stage 2, Commit 5 of 8. Drive the models/dependency and compilation subsystems under the 800-line file guardrail and clear their second-tier complexity offenders, preserving behaviour and the monkeypatch/import surface via reusable mixin classes. models/dependency/reference.py 1985 -> 702 (largest file in the repo): - DependencyReference now composes three mixins -- _ReferenceParseMixin (_reference_parse.py), _ReferenceUrlMixin (_reference_url.py), _ReferenceShorthandMixin (_reference_shorthand.py). The dataclass and composed symbol stay defined in reference.py so the patch surface is unchanged. - Leaf helpers/constants moved to _reference_util.py and re-exported from reference.py; this breaks the import cycle (mixins import the util, never the parent at module scope; back-references are TYPE_CHECKING-only). - parse_from_dict (C901) decomposed into shape-routed sub-parsers with two shared validators that genuinely de-duplicate the repeated alias/ref grammar. - models/validation.py: validate_apm_package PLR0911 9 -> 4 returns via a type-dispatch helper. compilation: - agents_compiler.py 1498 -> 785 (+_agents_emit, _agents_output mixins) - context_optimizer.py 1328 -> 548 (+_placement_solver, _pattern_matcher) - distributed_compiler.py 804 -> 658 (+_distributed_orphans) - link_resolver.py PLR0911 10 -> 8 returns via merged guards - moved methods that reference patched module globals (resolve_markdown_links, _logger, Path) route back through the origin module at call time so existing monkeypatches still apply. File-length backlog 19 -> 15. Complexity gate clean at final Stage-2 thresholds; thresholds flip in the final enforcement commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…splits Strangler Stage 2 second-tier complexity tightening, subsystem 6 of 7. Clears the 8 remaining >800-line offenders in core/, adapters/, cache/, utils/ and output/ via mixin/leaf-module splits, plus in-place complexity fixes - net -144 lines (genuine reduction despite the mandated splits). Length splits (all parents + new siblings <800): - core/auth.py 1142->670 (+_auth_support.py 532 mixin). Auth boundary preserved: get_bearer_provider/ls-remote methods stay in auth.py; the sibling reaches the ADO bearer provider via a new AuthResolver._ado_bearer_provider() accessor so Rule A stays clean. try_with_fallback de-duplicated (PLR0911 12->8, was failing on main). - core/script_runner.py 1135->686 (+_prompt_compiler.py 183, +_runtime_commands.py 302 mixin). - core/command_logger.py 836->221 (+_install_logger.py 599). InstallLogger re-exported lazily via PEP 562 __getattr__ to break the subclass<->base import cycle while preserving the command_logger._rich_*/CommandLogger patch surface. - cache/git_cache.py 830->599 (+_git_cache_bare.py 279 mixin). - utils/github_host.py 806->659 (+_github_host_artifactory.py 173). - adapters/client/base.py 866->434 (+_base_env.py 483 mixin). - adapters/client/copilot.py 1056->697 (+_copilot_env.py 388 mixin); CopilotClientAdapter override MRO preserved (_CopilotEnvMixin first). - output/formatters.py 994->484 (+_formatters_detail.py 465 mixin); removed _format_final_summary duplicate (byte-identical to _format_results_summary) + its duplicate test classes in two test files. Complexity-only (in place, no split): target_detection.py C901 via dict dispatch (+list-arg guards); primitives/parser.py PLR0911 12->3; cache/integrity.py PLR0911 9->8. Rule B routing added for every moved reference to a patched module global. No linter-gaming (no **kwargs, no complexity noqa; one obsolete noqa removed). Verification: ruff + format clean; complexity gate clean at final Stage-2 thresholds; pylint R0801 10.00/10; backlog 15->7; full unit+acceptance 16605 passed; targeted integration 3119 passed; auth-signals lint clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r 800 (#1078) Strangler Stage 2, Commit 7: drive the remaining seven >800-line files in the policy/ and marketplace/ subsystems under the 800-line guardrail via leaf-module and mixin splits that preserve every patched/public import surface, plus genuine PLR0911 decompositions. policy/ - discovery.py 1364->662: extract cache cluster to _discovery_cache.py and chain/host-pin helpers to _discovery_chain.py (Rule B routes the patched requests/subprocess/discover_policy/_write_cache globals). _fetch_github_contents PLR0911 11->2 via _parse_github_repo_ref / _decode_github_content / _call_github_api. - policy_checks.py 1134->693: extract MCP/compilation/manifest checks to _policy_checks_mcp.py; run_policy_checks PLR0911 9->5. _check_unmanaged_files + _MAX_UNMANAGED_SCAN_FILES kept in place to preserve a monkeypatch. - ci_checks.py run_baseline_checks PLR0911 11->6 (lazy lambda loop). - _constraint_pinning.py classify_unbounded_reason PLR0911 10->8. marketplace/ - yml_schema.py 1220->462: dataclasses to _yml_models.py (leaf), parsers to _yml_parsers.py (imports models, no cycle); dedup _parse_outputs. - builder.py 1130->506: report dataclasses to _builder_reports.py, resolve methods to _BuilderResolveMixin in _builder_resolve.py (Rule B for the 20x-patched urllib); dedup the _compute_diff plugin-SHA loop. - resolver.py 953->696: cross-repo-misconfig/matching cluster to _resolver_match.py; two PLR0911 9->5 via guard extraction. Public parse_marketplace_ref/get_marketplace_by_name/resolve_marketplace_plugin stay. - client.py 817->717: cache I/O cluster to _client_cache.py. Public fetch_marketplace/fetch_or_cache/clear_marketplace_cache/search_marketplace stay. - publisher.py 922->759: PublishState + dataclasses to _publish_state.py; _process_single_target PLR0911 9->8. - semver.py PLR0911 11->7 via _satisfies_caret; public API unchanged. All resulting files (parents + 10 new siblings) <800 (<=770). Whole-src >800 backlog 7->0. Full unit+acceptance 16605 passed; targeted integration 2747 passed; ruff/format/complexity(final thresholds)/R0801 10.00/auth-signals all clean. No threshold flip yet (enforcement is the final commit). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Final Strangler Stage 2 commit: flip the guardrails now that all
violations are fixed (commits for integration, deps, commands, models,
core/adapters, policy, marketplace landed earlier in this PR).
pyproject.toml [tool.ruff.lint]: tighten to Stage 2 targets
max-statements 200->120, max-args 15->12, max-branches 60->40,
max-returns 12->8, mccabe max-complexity 50->35; refresh the stale
Stage-1 roadmap comments.
.github/workflows/ci.yml: file-length guardrail MAX_LINES 2100->800;
refresh the stale comment.
CHANGELOG.md: record the threshold + guardrail tightening under Unreleased.
tests/integration/test_deps_resolver_{phase3b,resolution}.py: the new
max-args=12 surfaced a 15-arg `_make_dep_ref` test factory in both files.
Dropped the never-passed `explicit_scheme` param and grouped the three
cohesive Azure DevOps coordinates into a single `ado` triple (15->12 args)
- a parameter-object simplification, not an arg-count dodge.
Verification (CI-mirror, all green): ruff check src/ tests/; ruff format
--check; file-length guardrail (no src file >800); R0801 10.00/10;
auth-signals; YAML and relative_to guards; full unit+acceptance 16605
passed; targeted integration green. Whole-src >800 backlog is 0.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam
added
the
panel-review
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.
TL;DR
Strangler Stage 2 (issue #1078): tighten the six Ruff complexity thresholds to their Stage 2 targets and fix every resulting violation across the APM CLI, delivered as one PR with per-subsystem logical commits for reviewability. File-length goes straight to 800 (no Stage 2B follow-up).
Final thresholds (enforced in the last commit)
max-statementsmax-argsmax-branchesmax-returnsmax-complexityMAX_LINES)Approach
For files that are both a complexity offender and a file-length offender, extract over-complex function bodies into new sibling submodules — this reduces complexity and file length in one move. Public command surfaces and
@patchtest seams are preserved via module-level re-exports. Prefer genuine simplification (deduplication, shared helpers, dispatch tables, parameter objects) over mechanical code-moving.Progress
commands/install.pysplit (2085 -> 765 lines) into sixinstall/submodules. Monkeypatch seams preserved; inline local-bundle duplication removed (R0801 green). 16645 unit + acceptance and 927 install integration tests pass.pyproject.toml+ci.ymlthresholds (enforcement, last)Closes #1078.