From 55965a48ab826e91445002c0a21f9059c5ecd3f0 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 21 Aug 2026 12:46:43 -0400 Subject: [PATCH 1/4] fix(pycg)!: decide shard outcomes by convergence, not wall clock Sharded PyCG dropped shards that exceeded a wall-clock timeout, and a dropped shard contributed zero edges. Which shards ran slow depends on machine load and Ray scheduling, so byte-identical invocations produced different call graphs: three runs over one 2,364-file fixture gave 48,595 / 43,431 / 40,224 edges, an 11% spread with PyCG's own contribution swinging 44%. The clock bound was redundant. PyCG's fixpoint loop is while (max_iter < 0 or iter_cnt < max_iter) and not has_converged(): so --pycg-max-iter (default 50) already guarantees termination. Bounding it a second time by the clock added nothing but the load-dependence. A shard is now a runaway when its fixpoint stopped at max_iter instead of converging -- a function of the input alone. Adaptive decomposition is unchanged, since that is what recovers recall. A runaway that cannot be split further now keeps the edges it did derive: a capped fixpoint is a sound under-approximation, so discarding them was pure recall loss on top of the non-determinism. _PYCG_DECOMP_FLOOR drops 10 -> 1, because a floor of 10 left small runaways unsplittable and forced them down that path. Reading convergence needs care in two places, both covered by tests. Asking cg.has_converged() after analyze() returns is wrong: analyze runs a CallGraphProcessor pass past the loop, so a post-hoc call compares state that pass has already moved. The loop's last recorded value is also wrong: when the cap stops the loop the `and` short-circuits, leaving the False that admitted the final pass and mislabelling a shard that converged on exactly pass max_iter. So the cap is raised by one and the loop is cut off from inside the check -- one extra question, not an extra pass. BREAKING CHANGE: --pycg-shard-timeout is removed. It is the defect, and after this change it controls nothing. Use --pycg-max-iter to trade recall against runtime, deterministically. Note that --pycg-max-iter -1 now has no wall-clock net behind it, so a divergent shard can run indefinitely there. Closes #145 --- CHANGELOG.md | 23 ++ codeanalyzer/__main__.py | 24 +- codeanalyzer/core.py | 1 - codeanalyzer/options/options.py | 1 - .../semantic_analysis/pycg/pycg_analysis.py | 333 ++++++++++-------- test/test_pycg_shard_determinism.py | 236 +++++++++++++ test/test_pycg_sharding.py | 9 +- 7 files changed, 448 insertions(+), 179 deletions(-) create mode 100644 test/test_pycg_shard_determinism.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b9cc68e..f26a86c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **Sharded PyCG is deterministic again** (#145): `--pycg-shard` decided which + shards to keep by wall-clock timeout, so which shards survived depended on + machine load and Ray scheduling. A dropped shard contributed *zero* edges — + three byte-identical invocations over one 2,364-file fixture produced 48,595 / + 43,431 / 40,224 call edges, an 11% spread with PyCG's own contribution swinging + 44%. Shard outcomes are now decided by PyCG's own convergence + (`has_converged()`): a shard is a runaway when its fixpoint stopped at + `--pycg-max-iter` instead of converging, which is a function of the input + alone. Adaptive decomposition is unchanged — a runaway is still re-partitioned + at a tighter budget to recover recall — but a shard that cannot be split + further now keeps the edges it did produce instead of being discarded. A + capped fixpoint is a sound under-approximation, so those edges are real. + ### Changed +- **BREAKING: `--pycg-shard-timeout` is removed** (#145). It bounded PyCG's + fixpoint a second time, by the clock, after `--pycg-max-iter` had already + bounded it by iteration count — and that second bound is what made the output + load-dependent. PyCG terminates on its own at `--pycg-max-iter` (default 50), + so nothing is left unbounded at the default. Anyone passing + `--pycg-shard-timeout` must drop the flag; use `--pycg-max-iter` to trade + analysis depth against runtime. One caveat: `--pycg-max-iter -1` asks PyCG to + run to convergence with no cap, and there is no longer a wall-clock net behind + it, so a divergent shard can run indefinitely under that setting. - **BREAKING: the msgpack output format is removed** (#118, TS parity): the `--format msgpack` CLI choice, the `analysis.msgpack` artifact, the msgpack serialization mixin on schema models, and the `msgpack` dependency are gone. diff --git a/codeanalyzer/__main__.py b/codeanalyzer/__main__.py index 41e82b6..0986d64 100644 --- a/codeanalyzer/__main__.py +++ b/codeanalyzer/__main__.py @@ -255,21 +255,6 @@ def main( min=1, ), ] = 100, - pycg_shard_timeout: Annotated[ - int, - typer.Option( - "--pycg-shard-timeout", - help=( - "Per-shard wall-clock timeout in seconds when --pycg-shard is " - "active (default 120). A shard that exceeds this limit is skipped " - "gracefully. PyCG's fixpoint is bimodal: it either converges " - "quickly or diverges indefinitely, so the timeout acts as a final " - "safety net after the file-count ceiling. Set to 0 to disable. " - "POSIX only (macOS / Linux); ignored on Windows." - ), - min=0, - ), - ] = 120, pycg_shard_strategy: Annotated[ ShardStrategy, typer.Option( @@ -294,8 +279,12 @@ def main( "changing, but its access-path domain has no convergence bound, " "so heavy metaclass/mixin code (e.g. an ORM) can loop with each " "pass costing seconds. The cap returns a sound-but-incomplete " - "call graph instead of looping until the timeout kills it. " - "Set to -1 for PyCG's unbounded run-to-convergence behaviour." + "call graph instead of looping indefinitely. It is now the " + "only bound on a shard, and is what makes sharded output " + "reproducible, so lowering it trades recall for runtime " + "deterministically. Set to -1 for PyCG's unbounded " + "run-to-convergence behaviour -- with no wall-clock safety " + "net, so a divergent shard can then run indefinitely." ), min=-1, ), @@ -391,7 +380,6 @@ def main( verbosity=verbosity, pycg_shard=pycg_shard, pycg_shard_ceiling=pycg_shard_ceiling, - pycg_shard_timeout=pycg_shard_timeout, pycg_shard_strategy=pycg_shard_strategy, pycg_max_iter=pycg_max_iter, entrypoint_rules=tuple(entrypoint_rules or ()), diff --git a/codeanalyzer/core.py b/codeanalyzer/core.py index c6550e9..0918083 100644 --- a/codeanalyzer/core.py +++ b/codeanalyzer/core.py @@ -965,7 +965,6 @@ def _get_pycg_call_graph( skip_tests=self.skip_tests, shard=self.options.pycg_shard, shard_ceiling=self.options.pycg_shard_ceiling, - shard_timeout=self.options.pycg_shard_timeout, shard_strategy=self.options.pycg_shard_strategy, max_iter=self.options.pycg_max_iter, using_ray=self.using_ray, diff --git a/codeanalyzer/options/options.py b/codeanalyzer/options/options.py index c1fb362..4aac694 100644 --- a/codeanalyzer/options/options.py +++ b/codeanalyzer/options/options.py @@ -62,7 +62,6 @@ class AnalysisOptions: verbosity: int = 0 pycg_shard: bool = False pycg_shard_ceiling: int = 100 - pycg_shard_timeout: int = 120 pycg_shard_strategy: ShardStrategy = ShardStrategy.JEDI pycg_max_iter: int = 50 entrypoint_rules: Tuple[Path, ...] = () diff --git a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py index 32247ba..90c115b 100644 --- a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py +++ b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py @@ -47,7 +47,6 @@ import os import json # noqa: F401 import shutil -import signal import tempfile import time @@ -55,32 +54,6 @@ from pathlib import Path from typing import Any, Dict, Generator, List, Optional, Set, Tuple, Union - -@contextlib.contextmanager -def _shard_timeout(seconds: int) -> Generator[None, None, None]: - """Context manager that raises ``TimeoutError`` if the body runs longer than *seconds*. - - Uses SIGALRM on POSIX (macOS / Linux). On platforms without SIGALRM - (Windows) the context manager is a no-op — shards can still be bounded - by the file-count ceiling. - - Must be called from the main thread (SIGALRM restriction). - """ - if seconds <= 0 or not hasattr(signal, "SIGALRM"): - yield - return - - def _handler(signum: int, frame: object) -> None: - raise TimeoutError(f"shard timed out after {seconds}s") - - old_handler = signal.signal(signal.SIGALRM, _handler) - signal.alarm(seconds) - try: - yield - finally: - signal.alarm(0) - signal.signal(signal.SIGALRM, old_handler) - from codeanalyzer.schema.py_schema import PyCallEdge, PyModule from codeanalyzer.semantic_analysis.call_graph import iter_callables_in_symbol_table from codeanalyzer.semantic_analysis.pycg.pycg_exceptions import PyCGExceptions @@ -226,16 +199,76 @@ def _shard_symlink_root( os.close(lock_fd) +def _analyze_with_convergence(cg: Any) -> bool: + """Run ``cg.analyze()``; return whether its fixpoint converged. + + PyCG's loop is ``while (max_iter < 0 or iter_cnt < max_iter) and not + has_converged()``, so the convergence check runs *before* each pass and + never after the last one. Reading it correctly needs care in two places: + + * **Asking after ``analyze()`` returns does not work.** ``analyze`` runs a + ``CallGraphProcessor`` pass past the loop, so a post-hoc call compares + state that pass has already moved against the snapshot taken before it, + and can report divergence for a shard that converged. + * **The last recorded value alone does not work either.** When the cap is + what stops the loop, the ``and`` short-circuits and the check is skipped, + leaving the ``False`` that admitted the final pass — which mislabels a + shard that reached its fixpoint on exactly pass ``max_iter``. + + So raise the cap by one and cut the loop off from inside the check: at the + point PyCG would have stopped, evaluate convergence once more (this is + "did the final pass change anything") and then return ``True`` to stop the + loop before the extra pass can run. The verdict is a function of the input + alone, unlike the wall-clock timeout it replaces (#145). + """ + max_iter = cg.max_iter + if max_iter == 0: + # Degenerate: PyCG is asked for zero fixpoint passes, so the result is + # an under-approximation by configuration rather than by divergence. + # Re-splitting cannot improve it -- sub-shards get the same cap -- so + # calling it a runaway would only buy pointless re-analysis. + cg.analyze() + return True + + had_own_attr = "has_converged" in vars(cg) + original = cg.has_converged + last: List[bool] = [] + + def _recording() -> bool: + result = bool(original()) + last.append(result) + # PyCG would stop here on the cap without asking again; we asked, so + # stop the loop ourselves rather than let the raised cap buy a pass. + if max_iter >= 0 and len(last) > max_iter: + return True + return result + + cg.has_converged = _recording + if max_iter >= 0: + cg.max_iter = max_iter + 1 + try: + cg.analyze() + finally: + cg.max_iter = max_iter + if had_own_attr: + cg.has_converged = original + else: + del cg.has_converged + return last[-1] if last else True + + def _pycg_shard_worker( entry_points: List[str], package_dir: str, prefix: str, max_iter: int = -1, -) -> List[tuple]: +) -> Tuple[List[tuple], bool]: """Run PyCG on one shard; called in a Ray worker process. - Returns a list of ``(source, target, weight)`` tuples that the caller - converts to :class:`PyCallEdge` objects. This function is a plain + Returns ``(triples, converged)`` -- a list of ``(source, target, weight)`` + tuples that the caller converts to :class:`PyCallEdge` objects, and whether + PyCG reached its fixpoint rather than stopping at ``max_iter`` (#145). + This function is a plain module-level callable so it can be pickled by Ray without capturing any class-level state. *max_iter* caps PyCG's fixpoint passes (-1 = unbounded). """ @@ -271,7 +304,7 @@ def _pycg_shard_worker( max_iter=max_iter, operation="call-graph", ) - cg.analyze() + converged = _analyze_with_convergence(cg) edge_counts = _WorkerCounter() for src, dst in cg.output_edges(): @@ -280,7 +313,7 @@ def _pycg_shard_worker( dst = f"{prefix}.{dst}" edge_counts[(src, dst)] += 1 - return [(src, dst, count) for (src, dst), count in edge_counts.items()] + return [(src, dst, count) for (src, dst), count in edge_counts.items()], converged def _apply_pycg_posonly_patch() -> None: @@ -417,9 +450,6 @@ class PyCG: that exceed the 500-file ceiling. shard_ceiling: Maximum file count per shard. Shards exceeding this limit are skipped. Defaults to ``_PYCG_SHARD_CEILING`` (100). - shard_timeout: Per-shard wall-clock timeout in seconds. A shard that - exceeds this limit is skipped. 0 disables the timeout. Defaults - to ``_PYCG_SHARD_TIMEOUT`` (120). POSIX only; no-op on Windows. """ # PyCG's pointer analysis is practical only up to this many files. @@ -435,14 +465,6 @@ class PyCG: # conservative default; override via --pycg-shard-ceiling. _PYCG_SHARD_CEILING: int = 100 - # Per-shard wall-clock timeout (seconds). PyCG's fixpoint is bimodal: - # either it converges in seconds or it diverges and never finishes. - # This timeout acts as a final safety net after the file-count ceiling. - # 120 seconds is generous enough for any legitimately complex shard - # while still catching non-converging ones. Override via - # --pycg-shard-timeout. Set to 0 to disable. - _PYCG_SHARD_TIMEOUT: int = 120 - # Cap on PyCG's outer fixpoint passes. PyCG runs PostProcessor until the # def/scope/MRO state stops changing; its abstract domain (field-sensitive # access paths, no k-limiting or widening) has no ascending-chain bound, so @@ -455,11 +477,13 @@ class PyCG: # -1 restores PyCG's unbounded run-to-convergence behaviour. _PYCG_MAX_ITER: int = 50 - # Iterative decomposition of runaway (timed-out) shards: a shard that the - # wall-clock timeout kills is re-partitioned at half the budget and re-run, - # down to this file-count floor. Below the floor — or for an atomic import - # cycle that won't split — the residue falls back to Jedi-only coverage. - _PYCG_DECOMP_FLOOR: int = 10 + # Iterative decomposition of runaway shards: a shard whose fixpoint stopped + # at --pycg-max-iter instead of converging is re-partitioned at half the + # budget and re-run, down to this file-count floor. Below the floor — or + # for an atomic import cycle that won't split — the shard keeps the edges + # its capped fixpoint did derive. The floor is 1 because a lone divergent + # file is exactly the case worth isolating from its neighbours (#145). + _PYCG_DECOMP_FLOOR: int = 1 _PYCG_MAX_DECOMP_ROUNDS: int = 6 # Directory names that should never be fed to PyCG as entry points, nor @@ -479,7 +503,6 @@ def __init__( skip_tests: bool = True, shard: bool = False, shard_ceiling: Optional[int] = None, - shard_timeout: Optional[int] = None, shard_strategy: str = "jedi", max_iter: Optional[int] = None, using_ray: bool = False, @@ -490,9 +513,6 @@ def __init__( self.shard_ceiling = ( shard_ceiling if shard_ceiling is not None else self._PYCG_SHARD_CEILING ) - self.shard_timeout = ( - shard_timeout if shard_timeout is not None else self._PYCG_SHARD_TIMEOUT - ) self.max_iter = max_iter if max_iter is not None else self._PYCG_MAX_ITER # "jedi": partition the Jedi module graph (SCC + Louvain) so coupled # modules co-compute and few edges are severed (see shard_planner). @@ -610,9 +630,12 @@ def _run_pycg_batch( package_dir: Path, resolver: "_PyCGCallableResolver", prefix: str = "", - ) -> List[PyCallEdge]: + ) -> Tuple[List[PyCallEdge], bool]: """Run PyCG on *entry_points* with *package_dir* as the package root. + Returns ``(edges, converged)``; ``converged`` is False when PyCG stopped + at ``max_iter`` instead of reaching its fixpoint (#145). + *prefix* is a dot-separated path prepended to every edge name emitted by PyCG so that shard-relative names become project-relative. Pass ``""`` when *package_dir* is the project root (names already match). @@ -627,9 +650,7 @@ def _run_pycg_batch( max_iter=self.max_iter, operation="call-graph", ) - cg.analyze() - except TimeoutError: - raise # propagate directly so _build_sharded logs a clean timeout message + converged = _analyze_with_convergence(cg) except Exception as exc: raise PyCGExceptions.PyCGAnalysisError( f"PyCG analysis failed: {exc}" @@ -645,7 +666,7 @@ def _run_pycg_batch( return [ PyCallEdge(src=src, dst=dst, weight=count, prov=["pycg"]) for (src, dst), count in edge_counts.items() - ] + ], converged # ------------------------------------------------------------------ # Sharded analysis @@ -669,12 +690,22 @@ def _build_sharded_planned( PyCG's fixpoint diverges on heavy metaclass/mixin clusters, and a uniform ceiling would force *every* shard small (severing many edges) just to tame the few that run away. Instead we start coarse (low cut, high recall on - healthy code) and **only re-decompose the shards that time out**: each - runaway's files are re-partitioned at half the budget and re-run, down to - a floor. A runaway shard contributes zero edges, so splitting it recovers - almost all of them while paying cut on its internal seams alone. The - residue that still diverges at the floor (or is an atomic cycle that won't - split) falls back to Jedi-only coverage. + healthy code) and **only re-decompose the shards that did not converge**: + each runaway's files are re-partitioned at half the budget and re-run, + down to a floor. A smaller shard has a smaller fixpoint to reach, so + splitting recovers the edges the capped pass missed while paying cut on + its internal seams alone. + + A shard is a runaway when its fixpoint stopped at ``--pycg-max-iter`` + rather than converging — a function of the input, so the same project + decomposes the same way every run. This used to be a wall-clock + timeout, which made *which* shards were dropped depend on machine load + and Ray scheduling (#145). + + The residue that still diverges at the floor (or is an atomic cycle that + won't split) keeps the edges its capped fixpoint produced: a truncated + fixpoint is a sound under-approximation, so those edges are real and + dropping them was pure recall loss. """ self._resolver = resolver plan = plan_shards( @@ -716,18 +747,24 @@ def _build_sharded_planned( ) next_shards: List[List[str]] = [] - for rf in runaways: + for rf, partial in runaways: # Re-partition this runaway's files alone, at a tighter budget. # An atomic cycle (or a lone file) that won't shrink is - # irreducible — accept Jedi-only rather than loop forever. + # irreducible — keep the capped-fixpoint edges it did produce. + # A truncated fixpoint is a sound under-approximation, so + # keeping it strictly beats the previous behaviour of dropping + # the shard to zero edges (#145). Its sub-shards supersede it + # when it CAN be split, so the partial is only used here. sub_st = {f: symbol_table[f] for f in rf if f in symbol_table} if stop_decomposing or len(rf) <= 1: irreducible_files += len(rf) + all_edges.extend(partial) continue sub_plan = plan_shards(sub_st, jedi_edges, budget=next_budget) if len(sub_plan.shards) <= 1: - # did not actually split (one atomic SCC) — give up on it + # did not actually split (one atomic SCC) — keep its partial irreducible_files += len(rf) + all_edges.extend(partial) continue next_shards.extend(sub_plan.shards) @@ -742,8 +779,8 @@ def _build_sharded_planned( if irreducible_files: logger.warning( - "PyCG: %d file(s) in irreducibly-divergent shards fall back to " - "Jedi-only coverage", irreducible_files, + "PyCG: %d file(s) in irreducibly-divergent shards kept their " + "capped-fixpoint edges (sound under-approximation)", irreducible_files, ) result = self._coalesce_edges(all_edges) @@ -757,37 +794,50 @@ def _build_sharded_planned( def _run_fileset_shards_seq( self, shards: List[List[str]], - ) -> Tuple[List[PyCallEdge], List[List[str]]]: + ) -> Tuple[List[PyCallEdge], List[Tuple[List[str], List[PyCallEdge]]]]: """Run each file-set shard sequentially; return ``(edges, runaways)``. - A shard that times out or raises is returned in *runaways* (its file - list) for the caller to re-decompose; it contributes no edges. + A shard is a *runaway* when PyCG stopped at ``max_iter`` instead of + reaching its fixpoint, or when it raised. Both are deterministic + functions of the input -- unlike the wall-clock timeout this replaced, + which made the surviving edge set depend on machine load (#145). + + Each runaway carries the edges it *did* produce. A capped fixpoint is a + sound under-approximation, so if decomposition cannot split the shard + further the caller keeps that partial rather than discarding it. """ resolver = self._resolver edges_all: List[PyCallEdge] = [] - runaways: List[List[str]] = [] + runaways: List[Tuple[List[str], List[PyCallEdge]]] = [] with ProgressBar(len(shards), "Building call graph shards", item_label="shards") as progress: for files in shards: try: with _shard_symlink_root(files, self.project_dir) as (root, eps): - with _shard_timeout(self.shard_timeout): - edges = self._run_pycg_batch(eps, root, resolver, prefix="") - edges_all.extend(edges) - except (TimeoutError, PyCGExceptions.PyCGAnalysisError): - runaways.append(files) + edges, converged = self._run_pycg_batch( + eps, root, resolver, prefix="" + ) + if converged: + edges_all.extend(edges) + else: + runaways.append((files, edges)) + except PyCGExceptions.PyCGAnalysisError: + runaways.append((files, [])) progress.advance() return edges_all, runaways def _run_fileset_shards_ray( self, shards: List[List[str]], - ) -> Tuple[List[PyCallEdge], List[List[str]]]: + ) -> Tuple[List[PyCallEdge], List[Tuple[List[str], List[PyCallEdge]]]]: """Ray-parallel variant of :meth:`_run_fileset_shards_seq`. Each shard is materialised as a symlink mini-project up front (the trees - must outlive their remote tasks), submitted as a Ray task, and collected - against one wall-clock deadline — Ray workers cannot use SIGALRM, so the - timeout is enforced orchestrator-side. Timed-out/failed shards become - runaways; symlink trees are removed once the batch completes. + must outlive their remote tasks) and submitted as a Ray task. Every task + is collected -- there is no wall-clock deadline. A shard is a runaway + only when PyCG stopped at ``max_iter`` or the task raised, both + deterministic in the input (#145). The previous deadline cancelled + whichever tasks happened to be slowest, so the surviving edge set varied + with machine load: three runs over one fixture produced 48,595 / 43,431 / + 40,224 edges. """ import os import ray @@ -802,7 +852,7 @@ def _run_fileset_shards_ray( futures: List[Any] = [] meta: Dict[Any, List[str]] = {} # ObjectRef -> shard file list edges_all: List[PyCallEdge] = [] - runaways: List[List[str]] = [] + runaways: List[Tuple[List[str], List[PyCallEdge]]] = [] try: with ProgressBar(len(shards), "Building call graph shards (parallel)", item_label="shards") as progress: for files in shards: @@ -822,37 +872,25 @@ def _run_fileset_shards_ray( futures.append(fut) meta[fut] = files - deadline = ( - time.perf_counter() + float(self.shard_timeout) - if self.shard_timeout > 0 else None - ) + # No deadline: every task is collected. PyCG is bounded by + # `max_iter`, so a shard terminates on its own; bounding it again + # by the clock is what made the output load-dependent (#145). pending = list(futures) while pending: - if deadline is not None: - remaining = deadline - time.perf_counter() - if remaining <= 0: - break - else: - remaining = None - - ready, pending = ray.wait(pending, num_returns=1, timeout=remaining) - if not ready: - break - + ready, pending = ray.wait(pending, num_returns=1) fut = ready[0] try: - triples = ray.get(fut) - edges_all.extend( + triples, converged = ray.get(fut) + edges = [ PyCallEdge(src=s, dst=t, weight=w, prov=["pycg"]) for s, t, w in triples - ) + ] + if converged: + edges_all.extend(edges) + else: + runaways.append((meta[fut], edges)) except Exception: - runaways.append(meta[fut]) - progress.advance() - - for fut in pending: # exceeded the deadline - ray.cancel(fut, force=True) - runaways.append(meta[fut]) + runaways.append((meta[fut], [])) progress.advance() finally: for root in roots: @@ -911,19 +949,21 @@ def _build_sharded( continue prefix = self._package_prefix(pkg_root, self.project_dir) try: - with _shard_timeout(self.shard_timeout): - edges = self._run_pycg_batch(files, pkg_root, resolver, prefix=prefix) + # No wall-clock bound: PyCG terminates on `max_iter`, and + # timing out here made the edge set load-dependent (#145). + # A capped fixpoint still yields sound edges, so keep them. + edges, converged = self._run_pycg_batch( + files, pkg_root, resolver, prefix=prefix + ) all_edges.extend(edges) + if not converged: + logger.debug( + "PyCG shard '%s': fixpoint capped at max_iter", pkg_label, + ) logger.debug( "PyCG shard '%s': %d edges from %d files", pkg_label, len(edges), n, ) - except TimeoutError: - logger.warning( - "PyCG shard '%s' timed out after %ds — skipped", - pkg_label, self.shard_timeout, - ) - skipped += 1 except PyCGExceptions.PyCGAnalysisError as exc: logger.warning("PyCG shard '%s' failed — skipped: %s", pkg_label, exc) skipped += 1 @@ -931,9 +971,9 @@ def _build_sharded( if skipped: logger.warning( - "PyCG: %d shard(s) were skipped (exceeded %d-file ceiling, " - "%ds timeout, or failed)", - skipped, self.shard_ceiling, self.shard_timeout, + "PyCG: %d shard(s) were skipped (exceeded %d-file ceiling " + "or failed)", + skipped, self.shard_ceiling, ) # Merge duplicate (source, target) pairs that appear in multiple shards. @@ -961,10 +1001,9 @@ def _build_sharded( def _build_sharded_ray(self, shards: Dict[Path, List[str]]) -> List[PyCallEdge]: """Ray-parallel variant of the sequential shard loop. - All eligible shards are submitted as Ray remote tasks simultaneously. - ``ray.wait(timeout=shard_timeout)`` is used to collect results and - cancel stragglers — Ray workers cannot use SIGALRM, so the timeout is - enforced at the orchestrator level instead. + All eligible shards are submitted as Ray remote tasks simultaneously + and every one is collected. PyCG is bounded by ``max_iter``, so a + shard terminates on its own; there is no wall-clock deadline (#145). """ import os import ray @@ -999,59 +1038,36 @@ def _build_sharded_ray(self, shards: Dict[Path, List[str]]) -> List[PyCallEdge]: meta[fut] = (pkg_label, n) # Collect results one shard at a time so the progress bar ticks per - # completed shard. A single deadline governs the whole batch: tasks - # submitted simultaneously all have the same wall-clock budget. - deadline = ( - time.perf_counter() + float(self.shard_timeout) - if self.shard_timeout > 0 else None - ) + # completed shard. Every task is collected -- no deadline. PyCG is + # bounded by `max_iter`, so bounding it again by the clock only made + # the surviving edge set depend on machine load (#145). pending = list(futures) while pending: - if deadline is not None: - remaining = deadline - time.perf_counter() - if remaining <= 0: - break - else: - remaining = None - - ready, pending = ray.wait(pending, num_returns=1, timeout=remaining) - if not ready: - break # deadline reached before any new result - + ready, pending = ray.wait(pending, num_returns=1) fut = ready[0] pkg_label, n = meta[fut] try: - triples = ray.get(fut) + triples, converged = ray.get(fut) edges = [ PyCallEdge(src=s, dst=t, weight=w, prov=["pycg"]) for s, t, w in triples ] all_edges.extend(edges) logger.debug( - "PyCG shard '%s': %d edges from %d files (Ray)", + "PyCG shard '%s': %d edges from %d files (Ray)%s", pkg_label, len(edges), n, + "" if converged else " [fixpoint capped at max_iter]", ) except Exception as exc: logger.warning("PyCG shard '%s' failed — skipped: %s", pkg_label, exc) skipped += 1 progress.advance() - # Cancel any shards that did not complete before the deadline. - for fut in pending: - pkg_label, _ = meta[fut] - logger.warning( - "PyCG shard '%s' timed out after %ds — skipped", - pkg_label, self.shard_timeout, - ) - ray.cancel(fut, force=True) - skipped += 1 - progress.advance() - if skipped: logger.warning( - "PyCG: %d shard(s) were skipped (exceeded %d-file ceiling, " - "%ds timeout, or failed)", - skipped, self.shard_ceiling, self.shard_timeout, + "PyCG: %d shard(s) were skipped (exceeded %d-file ceiling " + "or failed)", + skipped, self.shard_ceiling, ) merged: Dict[tuple, PyCallEdge] = {} @@ -1147,7 +1163,12 @@ def build_call_graph_edges( # follow imports into those dependencies and explode the analysis. logger.info("PyCG: starting whole-project call graph analysis (%d files)", n_files) with _shard_symlink_root(entry_points, self.project_dir) as (root, eps): - edges = self._run_pycg_batch(eps, root, resolver, prefix="") + edges, converged = self._run_pycg_batch(eps, root, resolver, prefix="") + if not converged: + logger.warning( + "PyCG: fixpoint capped at max_iter=%d — edges are a sound " + "under-approximation", self.max_iter, + ) edges = _canonicalize_edges(edges) elapsed = time.perf_counter() - t0 diff --git a/test/test_pycg_shard_determinism.py b/test/test_pycg_shard_determinism.py new file mode 100644 index 0000000..33d8650 --- /dev/null +++ b/test/test_pycg_shard_determinism.py @@ -0,0 +1,236 @@ +"""Shard outcomes are decided by the input, never by the clock (#145). + +A shard that exceeded a wall-clock timeout used to be a "runaway" contributing +ZERO edges. Which shards ran slow depended on machine load, so identical runs +produced different call graphs — measured across one 2,364-file fixture: +48,595 / 43,431 / 40,224 edges for the same input and flags. + +PyCG's fixpoint is `while iter < max_iter and not has_converged()`, so a shard +terminates on its own. Asking `has_converged()` afterwards distinguishes +"reached fixpoint" from "hit the cap" — a function of the input alone. +""" +import inspect + +from codeanalyzer.semantic_analysis.pycg import pycg_analysis as pa + + +def _code_only(fn) -> str: + """Source with comment lines and the docstring stripped. + + The explanatory comments mention `deadline`; the assertions below are about + executable code, so compare against code alone. + """ + src = inspect.getsource(fn) + body = src.split('"""') + src = body[0] + "".join(body[2:]) if len(body) > 2 else src + return "\n".join( + ln for ln in src.splitlines() if not ln.strip().startswith("#") + ) + + +def test_no_wall_clock_deadline_governs_shard_collection(): + """Neither Ray collector may cancel work for being slow.""" + for fn in (pa.PyCG._run_fileset_shards_ray, pa.PyCG._build_sharded_ray): + src = _code_only(fn) + assert "ray.cancel" not in src, f"{fn.__name__}: a slow shard must not be cancelled" + assert "deadline" not in src, f"{fn.__name__}: no wall-clock deadline" + + +def test_module_has_no_wall_clock_bound_left(): + """Whole-module sweep: every path, not just the ones named above. + + The timeout was reachable from four collection paths; a check scoped to + one function would pass while another still dropped shards by the clock. + """ + src = "\n".join( + ln for ln in inspect.getsource(pa).splitlines() + if not ln.strip().startswith("#") + ) + for banned in ("SIGALRM", "signal.alarm", "ray.cancel", "shard_timeout"): + assert banned not in src, f"{banned} still reachable in pycg_analysis" + + +def test_sequential_runner_does_not_bound_shards_by_time(): + src = _code_only(pa.PyCG._run_fileset_shards_seq) + assert "_shard_timeout" not in src, "shard runs must not be wall-clock bounded" + assert "converged" in src, "runaway classification must use PyCG convergence" + + +def test_runaways_carry_their_partial_edges(): + """A capped fixpoint is a sound under-approximation — keep it, don't drop it.""" + src = inspect.getsource(pa.PyCG._run_fileset_shards_seq) + assert "runaways.append((files, edges))" in src + + loop = inspect.getsource(pa.PyCG._build_sharded_planned) + assert "all_edges.extend(partial)" in loop, ( + "an irreducible shard must contribute its capped-fixpoint edges, not zero" + ) + + +def test_decomposition_can_reach_a_single_file(): + """A floor of 10 files left small runaways unsplittable, forcing the drop path.""" + assert pa.PyCG._PYCG_DECOMP_FLOOR == 1 + + +def test_worker_reports_convergence(): + src = inspect.getsource(pa._pycg_shard_worker) + assert "_analyze_with_convergence" in src + assert "return [(src, dst, count) for (src, dst), count in edge_counts.items()], converged" in src + + +class _FakeCG: + """Stands in for PyCG's CallGraphGenerator, with its exact loop shape. + + Verbatim from PyCG 0.0.7 `CallGraphGenerator.analyze`: + while (self.max_iter < 0 or iter_cnt < self.max_iter) and ( + not self.has_converged() + ): ... + followed by a CallGraphProcessor pass that runs past the loop. + """ + + def __init__(self, converge_after, max_iter): + self.converge_after = converge_after + self.max_iter = max_iter + self.passes = 0 + self.post_loop_pass_ran = False + + def has_converged(self): + return self.passes >= self.converge_after + + def analyze(self): + iter_cnt = 0 + while (self.max_iter < 0 or iter_cnt < self.max_iter) and ( + not self.has_converged() + ): + self.passes += 1 + iter_cnt += 1 + self.post_loop_pass_ran = True # CallGraphProcessor + + +def test_convergence_true_when_fixpoint_reached(): + cg = _FakeCG(converge_after=3, max_iter=50) + assert pa._analyze_with_convergence(cg) is True + assert cg.passes == 3 + assert cg.post_loop_pass_ran + + +def test_convergence_false_when_capped_at_max_iter(): + cg = _FakeCG(converge_after=99, max_iter=5) + assert pa._analyze_with_convergence(cg) is False + assert cg.passes == 5 + + +def test_boundary_converges_exactly_at_the_cap(): + """Needing exactly max_iter passes is convergence, not a runaway.""" + cg = _FakeCG(converge_after=5, max_iter=5) + assert pa._analyze_with_convergence(cg) is True + + +def test_convergence_read_is_not_a_post_hoc_call(): + """The post-loop pass must not be able to flip the verdict. + + Reading `has_converged()` after `analyze()` returns would consult state the + CallGraphProcessor pass has already moved. This fake reports divergence + once that pass has run; the verdict must still be True. + """ + + class _MovesStateAfterLoop(_FakeCG): + def has_converged(self): + if self.post_loop_pass_ran: + return False + return super().has_converged() + + cg = _MovesStateAfterLoop(converge_after=2, max_iter=50) + assert pa._analyze_with_convergence(cg) is True + assert cg.has_converged() is False # a post-hoc call would have said runaway + + +def test_original_method_is_restored(): + cg = _FakeCG(converge_after=1, max_iter=50) + before = _FakeCG.has_converged + pa._analyze_with_convergence(cg) + assert "has_converged" not in vars(cg), "instance shim must be removed" + assert cg.has_converged.__func__ is before + + +def test_restored_even_when_analyze_raises(): + cg = _FakeCG(converge_after=1, max_iter=50) + cg.analyze = lambda: (_ for _ in ()).throw(RuntimeError("boom")) + try: + pa._analyze_with_convergence(cg) + except RuntimeError: + pass + assert "has_converged" not in vars(cg) + + +def test_raised_cap_does_not_buy_an_extra_pass(): + """The +1 exists to ask one more question, not to run one more pass.""" + cg = _FakeCG(converge_after=99, max_iter=5) + assert pa._analyze_with_convergence(cg) is False + assert cg.passes == 5, "a diverging shard must still stop at max_iter" + assert cg.max_iter == 5, "the cap must be restored" + + +def test_unbounded_max_iter_is_left_alone(): + cg = _FakeCG(converge_after=4, max_iter=-1) + assert pa._analyze_with_convergence(cg) is True + assert cg.passes == 4 + assert cg.max_iter == -1 + + +def test_zero_max_iter_runs_nothing(): + cg = _FakeCG(converge_after=99, max_iter=0) + assert pa._analyze_with_convergence(cg) is True, "nothing ran; nothing to re-split" + assert cg.passes == 0 + + +def test_irreducible_runaway_keeps_its_partial_edges(tmp_path): + """A shard that cannot be split contributes its edges, not zero. + + Previously such a shard fell back to "Jedi-only coverage" — the whole + reason a load-dependent timeout cost 5,164 edges between two identical + runs. A capped fixpoint is a sound under-approximation, so the edges it + did derive are real and belong in the output. + """ + from codeanalyzer.schema.py_schema import PyCallEdge, PyCallable, PyModule + from codeanalyzer.semantic_analysis.pycg.pycg_analysis import ( + PyCG, + _PyCGCallableResolver, + ) + + st, jedi = {}, [] + for i in range(6): + path = f"/proj/m{i}.py" + st[path] = PyModule( + file_path=path, + module_name=f"m{i}", + functions={"f": PyCallable(signature=f"m{i}.f", name="f", path=path)}, + ) + if i: + jedi.append( + PyCallEdge(src=f"m{i-1}.f", dst=f"m{i}.f", weight=1, prov=["jedi"]) + ) + + pycg = PyCG(tmp_path, shard_ceiling=6) + + def never_converges(shards): + """Every shard diverges, at every size — nothing is ever reducible.""" + runaways = [ + ( + files, + [ + PyCallEdge(src=f, dst="partial", weight=1, prov=["pycg"]) + for f in files + ], + ) + for files in shards + ] + return [], runaways + + pycg._run_fileset_shards_seq = never_converges + edges = pycg._build_sharded_planned(jedi, st, _PyCGCallableResolver(set())) + + srcs = {e.src for e in edges} + assert srcs == {f"/proj/m{i}.py" for i in range(6)}, ( + "every file's capped-fixpoint edges must survive decomposition bottoming out" + ) diff --git a/test/test_pycg_sharding.py b/test/test_pycg_sharding.py index eb902b2..72d32b1 100644 --- a/test/test_pycg_sharding.py +++ b/test/test_pycg_sharding.py @@ -47,7 +47,7 @@ def test_adaptive_decomposition_splits_runaways(tmp_path, monkeypatch): jedi.append(PyCallEdge(src=f"m{i-1}.f", dst=f"m{i}.f", weight=1, prov=["jedi"])) - # threshold >= the decomposition floor (10) so pieces can shrink enough to converge. + # threshold > 1 (the decomposition floor) so pieces can shrink enough to converge. pycg = PyCG(tmp_path, shard_ceiling=40) threshold = 12 # shards with > 12 files "diverge" rounds_seen = [] @@ -57,7 +57,10 @@ def fake_runner(shards): edges, runaways = [], [] for files in shards: if len(files) > threshold: - runaways.append(files) + # A non-converged shard carries the edges its capped fixpoint + # did derive; the caller keeps them only if it cannot split + # the shard further (#145). + runaways.append((files, [])) else: edges += [PyCallEdge(src=f, dst="x", weight=1, prov=["pycg"]) for f in files] @@ -100,7 +103,7 @@ def test_pycg_does_not_follow_into_in_tree_dependency(tmp_path): resolver = _PyCGCallableResolver(set()) entry_points = [str(app / "__init__.py"), str(app / "main.py")] with _shard_symlink_root(entry_points, proj) as (root, eps): - edges = pycg._run_pycg_batch(eps, root, resolver, prefix="") + edges, _converged = pycg._run_pycg_batch(eps, root, resolver, prefix="") nodes = {n for e in edges for n in (e.src, e.dst)} # bigdep is reachable as a ghost target ... From 98ecf0566353b5c2ad726e9b74d5e2776f2cf96e Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 24 Aug 2026 13:53:23 -0400 Subject: [PATCH 2/4] fix(pycg): keep capped shards instead of re-splitting them Exhausting --pycg-max-iter meant "runaway", so the shard was re-partitioned and re-analysed. That is backwards: hitting the cap means PyCG returned a sound under-approximation, and splitting such a shard makes the answer worse, because every cut severs the calls crossing it. Measured on one 100-file shard of a 2,364-file ORM-heavy project: bounding the fixpoint and keeping the shard whole gave 110,490 edges in 95s, where budget-driven halving gave 15,468 edges in 600s. Re-splitting also pays whole extra rounds of re-analysis -- with a low --pycg-max-iter every shard hits the cap, and one such run took 2h50m without finishing. A capped shard now contributes its edges directly; only a shard that raised is decomposed. Both classifications remain pure functions of the input, so the reproducibility 55965a4 restored is unaffected. Also documents that lowering --pycg-max-iter does not reliably bound runtime (per-pass cost dominates: a 26-file shard needs 5 passes and yields the identical 93 edges at 3 and at 50), adds a README section explaining why sharding exists and what it costs, and regenerates the --help block, which still advertised the --pycg-shard-timeout flag 55965a4 removed. Scope: this removes the load-dependent shard-dropping mechanism, the 11% effect #145 was filed for. Output is not yet byte-identical across runs -- a separate, ~100x smaller source remains in Jedi's overload resolution for open(), tracked as #146. Refs #145, #146 --- CHANGELOG.md | 49 +- README.md | 483 ++++++++---------- codeanalyzer/__main__.py | 11 +- .../semantic_analysis/pycg/pycg_analysis.py | 85 +-- test/test_pycg_shard_determinism.py | 33 +- 5 files changed, 323 insertions(+), 338 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f26a86c..3a545ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,21 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Fixed -- **Sharded PyCG is deterministic again** (#145): `--pycg-shard` decided which - shards to keep by wall-clock timeout, so which shards survived depended on - machine load and Ray scheduling. A dropped shard contributed *zero* edges — - three byte-identical invocations over one 2,364-file fixture produced 48,595 / - 43,431 / 40,224 call edges, an 11% spread with PyCG's own contribution swinging - 44%. Shard outcomes are now decided by PyCG's own convergence - (`has_converged()`): a shard is a runaway when its fixpoint stopped at - `--pycg-max-iter` instead of converging, which is a function of the input - alone. Adaptive decomposition is unchanged — a runaway is still re-partitioned - at a tighter budget to recover recall — but a shard that cannot be split - further now keeps the edges it did produce instead of being discarded. A - capped fixpoint is a sound under-approximation, so those edges are real. - ### Changed + +- **Exhausting `--pycg-max-iter` is no longer treated as a runaway** (#145). + Hitting the cap means PyCG returned a sound under-approximation — re-splitting + such a shard makes the answer *worse*, because every cut severs the calls + crossing it. Measured on one 100-file shard: bounding the fixpoint and keeping + the shard whole gave **110,490 edges in 95s**, where budget-driven halving gave + **15,468 in 600s**. Re-splitting also pays whole extra rounds of re-analysis — + with a low `--pycg-max-iter` every shard hits the cap, and one such run on a + 2,364-file project took **2h50m without finishing**. A capped shard now + contributes its edges directly; only a shard that *raised* is decomposed. Both + classifications remain pure functions of the input, so reproducibility is + unaffected. + - **BREAKING: `--pycg-shard-timeout` is removed** (#145). It bounded PyCG's fixpoint a second time, by the clock, after `--pycg-max-iter` had already bounded it by iteration count — and that second bound is what made the output @@ -55,6 +54,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 cp39 manylinux wheels for x86_64 and aarch64, so the source build stops happening there too. +### Fixed + +- **Sharded PyCG no longer drops shards by wall clock** (#145): `--pycg-shard` decided which + shards to keep by wall-clock timeout, so which shards survived depended on + machine load and Ray scheduling. A dropped shard contributed *zero* edges — + three byte-identical invocations over one 2,364-file fixture produced 48,595 / + 43,431 / 40,224 call edges, an 11% spread with PyCG's own contribution swinging + 44%. Shard outcomes are now decided by PyCG's own convergence + (`has_converged()`): a shard is a runaway when its fixpoint stopped at + `--pycg-max-iter` instead of converging, which is a function of the input + alone. Adaptive decomposition is unchanged — a runaway is still re-partitioned + at a tighter budget to recover recall — but a shard that cannot be split + further now keeps the edges it did produce instead of being discarded. A + capped fixpoint is a sound under-approximation, so those edges are real. + + **Scope:** this removes the load-dependent shard-dropping mechanism, which was + the 11% effect. Output is not yet byte-identical across runs: a separate, + much smaller source remains in Jedi's overload resolution for `open()` — + `f.read()` resolves to `_TextIOBase.read` or `_BufferedIOBase.read` depending + on the run, accounting for **0.1–0.3%** of edges on the Flask fixture. That is + tracked as #146 and is not addressed here. + ## [1.1.1] - 2026-07-27 ### Fixed diff --git a/README.md b/README.md index 92bf10d..a917d41 100644 --- a/README.md +++ b/README.md @@ -156,286 +156,173 @@ $ canpy --help Static Analysis on Python source code using Jedi, PyCG and Tree sitter. -╭─ Options ────────────────────────────────────────────────────────────────────╮ -│ --version Show the canpy │ -│ version and │ -│ exit. │ -│ --input -i Path to the │ -│ project root │ -│ directory (not │ -│ required for │ -│ --emit schema). │ -│ --output -o Output directory │ -│ for artifacts. │ -│ --format -f Output format │ -│ for --emit json: │ -│ json or msgpack. │ -│ [default: json] │ -│ --emit json │ -│ (analysis.json, │ -│ default) | neo4j │ -│ (graph.cypher or │ -│ live Bolt push) │ -│ | schema (the │ -│ Neo4j │ -│ schema.json │ -│ contract). │ -│ [default: json] │ -│ --app-name Logical │ -│ application name │ -│ for the graph │ -│ :PyApplication │ -│ anchor (default: │ -│ input dir name). │ -│ --neo4j-uri Push the graph │ -│ to a live Neo4j │ -│ over Bolt │ -│ (incremental); │ -│ omit to write │ -│ graph.cypher. │ -│ [env var: │ -│ NEO4J_URI] │ -│ --neo4j-user Neo4j username. │ -│ [env var: │ -│ NEO4J_USERNAME] │ -│ [default: neo4j] │ -│ --neo4j-password Neo4j password. │ -│ Prefer the env │ -│ var over the │ -│ flag (the flag │ -│ is visible in │ -│ shell history / │ -│ process list). │ -│ [env var: │ -│ NEO4J_PASSWORD] │ -│ [default: neo4j] │ -│ --neo4j-database Neo4j database │ -│ name (default: │ -│ server default). │ -│ [env var: │ -│ NEO4J_DATABASE] │ -│ --analysis-level -a Analysis depth: │ -│ [1<=x<=4] 1=symbol │ -│ table+Jedi call │ -│ graph, 2=+PyCG │ -│ call graph, │ -│ 3=+native │ -│ intraprocedural │ -│ dataflow │ -│ (CFG/PDG), │ -│ 4=+interprocedu… │ -│ SDG │ -│ (param/summary │ -│ edges, │ -│ alias-aware │ -│ DDG). │ -│ [default: 1] │ -│ --graphs Level 3+ only: │ -│ comma-separated │ -│ program-graph │ -│ sections to emit │ -│ (cfg, dfg, pdg, │ -│ sdg). Default: │ -│ cfg,dfg,pdg. │ -│ `dfg` emits the │ -│ PDG's data edges │ -│ only; `sdg` │ -│ requires -a 4. │ -│ [default: │ -│ cfg,dfg,pdg] │ -│ --graph-field-de… Level 3 only: │ -│ [x>=1] k-limit on │ -│ access-path │ -│ depth (x.f.g.h │ -│ with k=3 becomes │ -│ x.f.g.*). │ -│ Mandatory bound │ -│ — it is what │ -│ guarantees the │ -│ interprocedural │ -│ fixpoint │ -│ terminates. │ -│ [default: 3] │ -│ --ray --no-ray Enable Ray for │ -│ distributed │ -│ analysis. │ -│ [default: │ -│ no-ray] │ -│ --eager --lazy Enable eager or │ -│ lazy analysis. │ -│ Defaults to │ -│ lazy. │ -│ [default: lazy] │ -│ --skip-tests --include-tests Skip test files │ -│ in analysis. │ -│ [default: │ -│ skip-tests] │ -│ --no-venv --venv Skip virtualenv │ -│ creation and │ -│ dependency │ -│ installation; │ -│ resolve imports │ -│ against the │ -│ ambient Python │ -│ environment │ -│ instead. │ -│ [default: venv] │ -│ --file-name Analyze only the │ -│ specified file │ -│ (relative to │ -│ input │ -│ directory). │ -│ --cache-dir -c Directory to │ -│ store analysis │ -│ cache. Defaults │ -│ to │ -│ '.codeanalyzer' │ -│ in the input │ -│ directory. │ -│ --clear-cache --keep-cache Clear cache │ -│ after analysis. │ -│ By default, │ -│ cache is │ -│ retained. │ -│ [default: │ -│ keep-cache] │ -│ -v Increase │ -│ verbosity: -v, │ -│ -vv, -vvv │ -│ [default: 0] │ -│ --pycg-shard --no-pycg-shard Shard PyCG │ -│ call-graph │ -│ analysis by │ -│ Python package │ -│ (level 2 only). │ -│ When the project │ -│ exceeds the │ -│ 500-file │ -│ ceiling, PyCG is │ -│ run │ -│ independently │ -│ per top-level │ -│ package with │ -│ cross-package │ -│ imports treated │ -│ as ghost nodes. │ -│ Without this │ -│ flag, projects │ -│ over the ceiling │ -│ fall back to │ -│ Jedi-only edges. │ -│ [default: │ -│ no-pycg-shard] │ -│ --pycg-shard-cei… Maximum files │ -│ [x>=1] per shard when │ -│ --pycg-shard is │ -│ active (default │ -│ 100). Shards │ -│ exceeding this │ -│ limit are │ -│ skipped; their │ -│ call edges are │ -│ omitted from the │ -│ call graph (Jedi │ -│ edges for those │ -│ packages are │ -│ still included). │ -│ Lower values are │ -│ safer for │ -│ packages with │ -│ deep class │ -│ hierarchies or │ -│ heavy import │ -│ graphs. │ -│ [default: 100] │ -│ --pycg-shard-tim… Per-shard │ -│ [x>=0] wall-clock │ -│ timeout in │ -│ seconds when │ -│ --pycg-shard is │ -│ active (default │ -│ 120). A shard │ -│ that exceeds │ -│ this limit is │ -│ skipped │ -│ gracefully. │ -│ PyCG's fixpoint │ -│ is bimodal: it │ -│ either converges │ -│ quickly or │ -│ diverges │ -│ indefinitely, so │ -│ the timeout acts │ -│ as a final │ -│ safety net after │ -│ the file-count │ -│ ceiling. Set to │ -│ 0 to disable. │ -│ POSIX only │ -│ (macOS / Linux); │ -│ ignored on │ -│ Windows. │ -│ [default: 120] │ -│ --pycg-shard-str… How --pycg-shard │ -│ groups files │ -│ (level 2 only). │ -│ 'jedi' (default) │ -│ partitions the │ -│ Jedi │ -│ module-dependen… │ -│ graph (SCC + │ -│ Louvain) so │ -│ tightly-coupled │ -│ modules │ -│ co-compute and │ -│ few call edges │ -│ are severed │ -│ between shards; │ -│ import cycles │ -│ are never split. │ -│ 'package' uses │ -│ the legacy │ -│ one-shard-per-p… │ -│ grouping. │ -│ [default: jedi] │ -│ --pycg-max-iter Cap on PyCG's │ -│ [x>=-1] fixpoint passes │ -│ per │ -│ shard/project │ -│ (level 2; │ -│ default 50). │ -│ PyCG iterates │ -│ until its │ -│ points-to state │ -│ stops changing, │ -│ but its │ -│ access-path │ -│ domain has no │ -│ convergence │ -│ bound, so heavy │ -│ metaclass/mixin │ -│ code (e.g. an │ -│ ORM) can loop │ -│ with each pass │ -│ costing seconds. │ -│ The cap returns │ -│ a │ -│ sound-but-incom… │ -│ call graph │ -│ instead of │ -│ looping until │ -│ the timeout │ -│ kills it. Set to │ -│ -1 for PyCG's │ -│ unbounded │ -│ run-to-converge… │ -│ behaviour. │ -│ [default: 50] │ -│ --help Show this │ -│ message and │ -│ exit. │ -╰──────────────────────────────────────────────────────────────────────────────╯ +╭─ Options ────────────────────────────────────────────────────────────────────────────────────────╮ +│ --version Show the canpy version and │ +│ exit. │ +│ --input -i Path to the project root │ +│ directory (not required for │ +│ --emit schema). │ +│ --output -o Output directory for │ +│ artifacts. │ +│ --format -f Output format for --emit │ +│ json: json. │ +│ [default: json] │ +│ --emit Output target: json │ +│ (analysis.json, default) | │ +│ neo4j (graph.cypher or live │ +│ Bolt push) | schema (the │ +│ Neo4j schema.json │ +│ contract). │ +│ [default: json] │ +│ --app-name Logical application name │ +│ for the graph │ +│ :PyApplication anchor │ +│ (default: input dir name). │ +│ --neo4j-uri Push the graph to a live │ +│ Neo4j over Bolt │ +│ (incremental); omit to │ +│ write graph.cypher. │ +│ [env var: NEO4J_URI] │ +│ --neo4j-user Neo4j username. │ +│ [env var: NEO4J_USERNAME] │ +│ [default: neo4j] │ +│ --neo4j-password Neo4j password. Prefer the │ +│ env var over the flag (the │ +│ flag is visible in shell │ +│ history / process list). │ +│ [env var: NEO4J_PASSWORD] │ +│ [default: neo4j] │ +│ --neo4j-database Neo4j database name │ +│ (default: server default). │ +│ [env var: NEO4J_DATABASE] │ +│ --analysis-level -a [1<=x<=4] Analysis depth: 1=symbol │ +│ table+Jedi call graph, │ +│ 2=+PyCG call graph, │ +│ 3=+native intraprocedural │ +│ dataflow (CFG/PDG), │ +│ 4=+interprocedural SDG │ +│ (param/summary edges, │ +│ alias-aware DDG). │ +│ [default: (1)] │ +│ --graphs Level 3+ only: │ +│ comma-separated │ +│ program-graph sections to │ +│ emit (cfg, dfg, pdg, sdg). │ +│ Default: cfg,dfg,pdg. `dfg` │ +│ emits the PDG's data edges │ +│ only; `sdg` requires -a 4. │ +│ Incompatible with --emit │ +│ neo4j (always full-depth). │ +│ [default: (cfg,dfg,pdg)] │ +│ --graph-field-depth [x>=1] Level 3 only: k-limit on │ +│ access-path depth (x.f.g.h │ +│ with k=3 becomes x.f.g.*). │ +│ Mandatory bound — it is │ +│ what guarantees the │ +│ interprocedural fixpoint │ +│ terminates. │ +│ [default: 3] │ +│ --ray --no-ray Enable Ray for distributed │ +│ analysis. │ +│ [default: no-ray] │ +│ --eager --lazy Enable eager or lazy │ +│ analysis. Defaults to lazy. │ +│ [default: lazy] │ +│ --skip-tests --include-tests Skip test files in │ +│ analysis. │ +│ [default: skip-tests] │ +│ --no-venv --venv Skip virtualenv creation │ +│ and dependency │ +│ installation; resolve │ +│ imports against the ambient │ +│ Python environment instead. │ +│ [default: venv] │ +│ --file-name Analyze only the specified │ +│ file (relative to input │ +│ directory). │ +│ --cache-dir -c Directory to store analysis │ +│ cache. Defaults to │ +│ '.codeanalyzer' in the │ +│ input directory. │ +│ --clear-cache --keep-cache Clear cache after analysis. │ +│ By default, cache is │ +│ retained. │ +│ [default: keep-cache] │ +│ -v Increase verbosity: -v, │ +│ -vv, -vvv │ +│ [default: 0] │ +│ --pycg-shard --no-pycg-shard Shard PyCG call-graph │ +│ analysis by Python package │ +│ (level 2 only). When the │ +│ project exceeds the │ +│ 500-file ceiling, PyCG is │ +│ run independently per │ +│ top-level package with │ +│ cross-package imports │ +│ treated as ghost nodes. │ +│ Without this flag, projects │ +│ over the ceiling fall back │ +│ to Jedi-only edges. │ +│ [default: no-pycg-shard] │ +│ --pycg-shard-ceiling [x>=1] Maximum files per shard │ +│ when --pycg-shard is active │ +│ (default 100). Shards │ +│ exceeding this limit are │ +│ skipped; their call edges │ +│ are omitted from the call │ +│ graph (Jedi edges for those │ +│ packages are still │ +│ included). Lower values are │ +│ safer for packages with │ +│ deep class hierarchies or │ +│ heavy import graphs. │ +│ [default: 100] │ +│ --pycg-shard-strategy How --pycg-shard groups │ +│ files (level 2 only). │ +│ 'jedi' (default) partitions │ +│ the Jedi module-dependency │ +│ graph (SCC + Louvain) so │ +│ tightly-coupled modules │ +│ co-compute and few call │ +│ edges are severed between │ +│ shards; import cycles are │ +│ never split. 'package' uses │ +│ the legacy │ +│ one-shard-per-package-dire… │ +│ grouping. │ +│ [default: jedi] │ +│ --pycg-max-iter [x>=-1] Cap on PyCG's fixpoint │ +│ passes per shard/project │ +│ (level 2; default 50). PyCG │ +│ iterates until its │ +│ points-to state stops │ +│ changing, but its │ +│ access-path domain has no │ +│ convergence bound, so heavy │ +│ metaclass/mixin code (e.g. │ +│ an ORM) can loop with each │ +│ pass costing seconds. The │ +│ cap returns a │ +│ sound-but-incomplete call │ +│ graph instead of looping │ +│ indefinitely. Lowering it │ +│ does not reliably bound │ +│ runtime — per-pass cost │ +│ dominates — and a low cap │ +│ makes nearly every shard │ +│ hit it. Set to -1 for │ +│ unbounded │ +│ run-to-convergence, which │ +│ has no wall-clock net, so a │ +│ divergent shard can then │ +│ run indefinitely. │ +│ [default: 50] │ +│ --entrypoint-rules Extra entrypoint rules file │ +│ (YAML). Repeatable; merges │ +│ with the shipped rules. A │ +│ malformed file is an error. │ +│ --help Show this message and exit. │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ ``` @@ -509,6 +396,42 @@ levels are cumulative and additive — `analysis.json(-a 1) ⊆ … ⊆ analysis unless requested. Flag gating: `--graphs sdg` requires `-a 4`; `--graphs cfg,dfg,pdg` and `--graph-field-depth` require `-a 3`. +### Why sharding? + +
+Why level 2 splits large projects into chunks, and what that costs + +**What it's computing.** The analyzer builds a call graph: who calls whom. That's easy when code +says `foo()`, and hard in Python, where a variable might hold any of several things depending on how +the program ran. So PyCG guesses what every variable could point to, then repeatedly refines those +guesses until they stop changing. Every guess can affect every other guess, which is why the work +explodes. + +A measurement on a real project makes it concrete. Same project, same machine: + +- a 26-file chunk: 0.2 seconds +- a 100-file chunk: 108 seconds — and with the old settings, over 23 minutes without ever finishing + +Four times the files, but hundreds of times the cost. Odoo has 2,364 files. Analyzed in one piece it +would simply never finish. + +So sharding cuts the project into chunks small enough to complete, analyzes each separately, and +merges the results. + +The cost of doing that is that calls crossing a boundary get lost. If chunk A calls into chunk B, +neither chunk sees the whole picture, so that call vanishes from the graph. On this project that's +about 75% of call edges severed — a real accuracy price paid for being able to finish at all. + +Which is why the chunking isn't random: a planner clusters closely-related files into the same +chunk, so most calls stay inside a boundary and fewer are lost. + +The analogy that fits: proofreading a long novel for continuity errors. One chapter at a time is +fast, but you'll miss contradictions between chapters. Checking every chapter against every other +catches everything and takes essentially forever. Sharding is choosing chapter-sized pieces, and +grouping the chapters that reference each other. + +
+ ## Architecture & Tooling The dataflow substrate is hand-built from the standard library so every graph node joins back to a diff --git a/codeanalyzer/__main__.py b/codeanalyzer/__main__.py index 0986d64..74be02f 100644 --- a/codeanalyzer/__main__.py +++ b/codeanalyzer/__main__.py @@ -279,12 +279,11 @@ def main( "changing, but its access-path domain has no convergence bound, " "so heavy metaclass/mixin code (e.g. an ORM) can loop with each " "pass costing seconds. The cap returns a sound-but-incomplete " - "call graph instead of looping indefinitely. It is now the " - "only bound on a shard, and is what makes sharded output " - "reproducible, so lowering it trades recall for runtime " - "deterministically. Set to -1 for PyCG's unbounded " - "run-to-convergence behaviour -- with no wall-clock safety " - "net, so a divergent shard can then run indefinitely." + "call graph instead of looping indefinitely. Lowering it does " + "not reliably bound runtime — per-pass cost dominates — and a " + "low cap makes nearly every shard hit it. Set to -1 for " + "unbounded run-to-convergence, which has no wall-clock net, so " + "a divergent shard can then run indefinitely." ), min=-1, ), diff --git a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py index 90c115b..c2b819b 100644 --- a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py +++ b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py @@ -473,8 +473,12 @@ class PyCG: # comes, takes many passes. A finite cap turns "loop until killed" into a # sound-but-incomplete result that still returns the edges found so far. # 50 is generous — well-behaved code converges in well under 20 passes — - # while bounding the pathological case. Override via --pycg-max-iter; - # -1 restores PyCG's unbounded run-to-convergence behaviour. + # while bounding the pathological case. Note that lowering it does NOT + # reliably bound runtime: per-pass cost dominates, and a low cap makes + # nearly every shard hit it (measured: a 26-file shard needs 5 passes and + # yields the identical 93 edges at 3 and at 50). Override via + # --pycg-max-iter; -1 restores PyCG's unbounded run-to-convergence + # behaviour, with no wall-clock net behind it. _PYCG_MAX_ITER: int = 50 # Iterative decomposition of runaway shards: a shard whose fixpoint stopped @@ -690,17 +694,20 @@ def _build_sharded_planned( PyCG's fixpoint diverges on heavy metaclass/mixin clusters, and a uniform ceiling would force *every* shard small (severing many edges) just to tame the few that run away. Instead we start coarse (low cut, high recall on - healthy code) and **only re-decompose the shards that did not converge**: - each runaway's files are re-partitioned at half the budget and re-run, - down to a floor. A smaller shard has a smaller fixpoint to reach, so - splitting recovers the edges the capped pass missed while paying cut on - its internal seams alone. - - A shard is a runaway when its fixpoint stopped at ``--pycg-max-iter`` - rather than converging — a function of the input, so the same project - decomposes the same way every run. This used to be a wall-clock - timeout, which made *which* shards were dropped depend on machine load - and Ray scheduling (#145). + healthy code) and **only re-decompose the shards that FAILED**: a shard + that raised has its files re-partitioned at half the budget and re-run, + down to a floor. + + Exhausting ``--pycg-max-iter`` is deliberately NOT a runaway, because + splitting such a shard makes the result *worse*: every cut severs the + calls crossing it. Measured on one 100-file shard, bounding the + fixpoint and keeping the shard whole gave 110,490 edges in 95s, where + budget-driven halving gave 15,468 in 600s. Re-splitting also costs + whole extra rounds of re-analysis — with a low ``--pycg-max-iter`` every + shard hits the cap, and one such run took 2h50m without finishing. + + Runaway classification stays a function of the input, so the + reproducibility the wall-clock timeout cost us is preserved (#145). The residue that still diverges at the floor (or is an atomic cycle that won't split) keeps the edges its capped fixpoint produced: a truncated @@ -726,7 +733,7 @@ def _build_sharded_planned( all_edges: List[PyCallEdge] = [] shards = plan.shards budget = self.shard_ceiling - converged_total = 0 + accepted_total = 0 irreducible_files = 0 round_no = 0 @@ -737,7 +744,7 @@ def _build_sharded_planned( logger.info("PyCG: %s", label) edges, runaways = runner(shards) all_edges.extend(edges) - converged_total += len(shards) - len(runaways) + accepted_total += len(shards) - len(runaways) if not runaways: break @@ -785,9 +792,9 @@ def _build_sharded_planned( result = self._coalesce_edges(all_edges) logger.info( - "PyCG: %d edges from %d converged shard(s) over %d round(s) " + "PyCG: %d edges from %d accepted shard(s) over %d round(s) " "(%d before dedup, Jedi-planned%s)", - len(result), converged_total, round_no + 1, len(all_edges), + len(result), accepted_total, round_no + 1, len(all_edges), ", Ray-parallel" if self.using_ray else "", ) return result @@ -797,14 +804,20 @@ def _run_fileset_shards_seq( ) -> Tuple[List[PyCallEdge], List[Tuple[List[str], List[PyCallEdge]]]]: """Run each file-set shard sequentially; return ``(edges, runaways)``. - A shard is a *runaway* when PyCG stopped at ``max_iter`` instead of - reaching its fixpoint, or when it raised. Both are deterministic - functions of the input -- unlike the wall-clock timeout this replaced, - which made the surviving edge set depend on machine load (#145). + A shard is a *runaway* only when it RAISED. Exhausting ``max_iter`` is + NOT a runaway: it means PyCG returned a sound under-approximation, and + re-splitting that shard makes the answer worse rather than better. - Each runaway carries the edges it *did* produce. A capped fixpoint is a - sound under-approximation, so if decomposition cannot split the shard - further the caller keeps that partial rather than discarding it. + Every cut severs the calls crossing it, so a split shard sees strictly + less. Measured on one 100-file shard: bounding the fixpoint and keeping + the shard whole gave 110,490 edges in 95s, where budget-driven halving + gave 15,468 edges in 600s. Re-splitting also pays whole extra rounds of + re-analysis -- with a low ``--pycg-max-iter`` every shard hits the cap, + and one such run took 2h50m without finishing. + + Both classifications remain pure functions of the input, so the + reproducibility this replaced the wall-clock timeout for is unaffected + (#145). """ resolver = self._resolver edges_all: List[PyCallEdge] = [] @@ -816,10 +829,14 @@ def _run_fileset_shards_seq( edges, converged = self._run_pycg_batch( eps, root, resolver, prefix="" ) - if converged: - edges_all.extend(edges) - else: - runaways.append((files, edges)) + # Converged or capped, the edges are sound -- keep them. + edges_all.extend(edges) + if not converged: + logger.debug( + "PyCG shard: fixpoint capped at max_iter=%d " + "(expected; edges are a sound under-approximation)", + self.max_iter, + ) except PyCGExceptions.PyCGAnalysisError: runaways.append((files, [])) progress.advance() @@ -885,10 +902,14 @@ def _run_fileset_shards_ray( PyCallEdge(src=s, dst=t, weight=w, prov=["pycg"]) for s, t, w in triples ] - if converged: - edges_all.extend(edges) - else: - runaways.append((meta[fut], edges)) + # Converged or capped, the edges are sound -- keep them. + edges_all.extend(edges) + if not converged: + logger.debug( + "PyCG shard: fixpoint capped at max_iter=%d " + "(expected; sound under-approximation)", + self.max_iter, + ) except Exception: runaways.append((meta[fut], [])) progress.advance() diff --git a/test/test_pycg_shard_determinism.py b/test/test_pycg_shard_determinism.py index 33d8650..f55542b 100644 --- a/test/test_pycg_shard_determinism.py +++ b/test/test_pycg_shard_determinism.py @@ -53,17 +53,38 @@ def test_module_has_no_wall_clock_bound_left(): def test_sequential_runner_does_not_bound_shards_by_time(): src = _code_only(pa.PyCG._run_fileset_shards_seq) assert "_shard_timeout" not in src, "shard runs must not be wall-clock bounded" - assert "converged" in src, "runaway classification must use PyCG convergence" + assert "max_iter" in src, "the cap must be documented as expected, not a runaway" -def test_runaways_carry_their_partial_edges(): - """A capped fixpoint is a sound under-approximation — keep it, don't drop it.""" - src = inspect.getsource(pa.PyCG._run_fileset_shards_seq) - assert "runaways.append((files, edges))" in src +def test_capped_shards_are_kept_not_re_split(): + """Exhausting max_iter is the expected case, not a runaway. + --pycg-max-iter is a cost bound with a low default, so nearly every shard + hits it. Treating that as a runaway re-analysed the whole project up to + _PYCG_MAX_DECOMP_ROUNDS times (one measured run: 2h50m, unfinished), and + splitting destroys edges besides -- 110,490 edges in 95s keeping a shard + whole, versus 15,468 in 600s under budget-driven halving. + """ + for fn in (pa.PyCG._run_fileset_shards_seq, pa.PyCG._run_fileset_shards_ray): + src = _code_only(fn) + assert "runaways.append((files, edges))" not in src, ( + f"{fn.__name__}: a capped shard must not become a runaway" + ) + assert "runaways.append((meta[fut], edges))" not in src, ( + f"{fn.__name__}: a capped shard must not become a runaway" + ) + assert "edges_all.extend(edges)" in src, ( + f"{fn.__name__}: capped edges must still be kept" + ) + + +def test_failed_shards_are_still_re_split(): + """A shard that RAISED is still worth decomposing.""" + seq = _code_only(pa.PyCG._run_fileset_shards_seq) + assert "runaways.append((files, []))" in seq loop = inspect.getsource(pa.PyCG._build_sharded_planned) assert "all_edges.extend(partial)" in loop, ( - "an irreducible shard must contribute its capped-fixpoint edges, not zero" + "an irreducible shard must contribute whatever edges it produced, not zero" ) From 7a71a6d784398ff5f558135545fe865a5e7fb055 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 24 Aug 2026 21:39:24 -0400 Subject: [PATCH 3/4] docs(pycg): drop unreliable timings, correct the termination claim Two corrections to claims made in this branch. Timings: the same 100-file shard, same --pycg-max-iter, same fence and same machine, has completed in ~8 minutes on one run and failed to complete in 81 minutes on another (98% CPU throughout, resident set flat, no output). PyCG's cost on this workload is bimodal, not merely noisy, so the wall-clock figures previously quoted (95s / 600s) are not measurements anyone should rely on -- and the halving figure was a harness cap rather than a completion. The argument for keeping a capped shard whole rests on edge counts, which are deterministic properties of the analysis: 110,490 edges whole versus 15,468 halved, because every cut severs the calls crossing it. That claim stands without any timing. Termination: 55965a4 argued the wall-clock bound was redundant because --pycg-max-iter already guarantees termination. That is wrong. The cap bounds fixpoint *iterations* and is only consulted at pass boundaries, so a single pathological pass escapes it entirely -- which is what the 81-minute run was doing. Removing the timeout removes the only wall-clock bound that existed. The timeout had to go because it made the output depend on machine load, but nothing replaces it, and the docs now say so instead of implying otherwise. Refs #145 --- CHANGELOG.md | 17 ++++++++++++++--- .../semantic_analysis/pycg/pycg_analysis.py | 13 +++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a545ac..d9154dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,8 +13,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Hitting the cap means PyCG returned a sound under-approximation — re-splitting such a shard makes the answer *worse*, because every cut severs the calls crossing it. Measured on one 100-file shard: bounding the fixpoint and keeping - the shard whole gave **110,490 edges in 95s**, where budget-driven halving gave - **15,468 in 600s**. Re-splitting also pays whole extra rounds of re-analysis — + the shard whole gave **110,490 edges**, where budget-driven halving of the same + shard gave **15,468**. (Edge counts are deterministic; wall-clock on this + workload is not — the same shard has taken 8 minutes and >81 minutes on the + same machine — so no timings are quoted.) Re-splitting also pays extra rounds + of re-analysis — with a low `--pycg-max-iter` every shard hits the cap, and one such run on a 2,364-file project took **2h50m without finishing**. A capped shard now contributes its edges directly; only a shard that *raised* is decomposed. Both @@ -64,7 +67,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 44%. Shard outcomes are now decided by PyCG's own convergence (`has_converged()`): a shard is a runaway when its fixpoint stopped at `--pycg-max-iter` instead of converging, which is a function of the input - alone. Adaptive decomposition is unchanged — a runaway is still re-partitioned + alone. + + **`--pycg-max-iter` is not a termination guarantee.** It bounds fixpoint + *iterations* and is only consulted at pass boundaries, so a single + pathological pass escapes it — one shard ran >81 minutes at `max_iter=3` + without completing, while the identical shard completed in ~8 minutes on + another run. Removing the wall-clock timeout therefore removes the only + wall-clock bound that existed; that bound was non-deterministic and had to go, + but nothing replaces it yet. Adaptive decomposition is unchanged — a runaway is still re-partitioned at a tighter budget to recover recall — but a shard that cannot be split further now keeps the edges it did produce instead of being discarded. A capped fixpoint is a sound under-approximation, so those edges are real. diff --git a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py index c2b819b..4cfc149 100644 --- a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py +++ b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py @@ -701,8 +701,11 @@ def _build_sharded_planned( Exhausting ``--pycg-max-iter`` is deliberately NOT a runaway, because splitting such a shard makes the result *worse*: every cut severs the calls crossing it. Measured on one 100-file shard, bounding the - fixpoint and keeping the shard whole gave 110,490 edges in 95s, where - budget-driven halving gave 15,468 in 600s. Re-splitting also costs + fixpoint and keeping the shard whole gave 110,490 edges, where + budget-driven halving of the same shard gave 15,468. (Edge counts are + deterministic; wall-clock on this workload is not -- the same shard has + taken 8 minutes and >81 minutes on the same machine, so timings are + deliberately not quoted.) Re-splitting also costs whole extra rounds of re-analysis — with a low ``--pycg-max-iter`` every shard hits the cap, and one such run took 2h50m without finishing. @@ -810,8 +813,10 @@ def _run_fileset_shards_seq( Every cut severs the calls crossing it, so a split shard sees strictly less. Measured on one 100-file shard: bounding the fixpoint and keeping - the shard whole gave 110,490 edges in 95s, where budget-driven halving - gave 15,468 edges in 600s. Re-splitting also pays whole extra rounds of + the shard whole gave 110,490 edges, where budget-driven halving of the + same shard gave 15,468. (Edge counts are deterministic; wall-clock is + not -- the same shard has taken 8 minutes and >81 minutes on the same + machine.) Re-splitting also pays whole extra rounds of re-analysis -- with a low ``--pycg-max-iter`` every shard hits the cap, and one such run took 2h50m without finishing. From cfd4d758792a5f94b2b83a34e183c6b4e7df3455 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Tue, 25 Aug 2026 11:13:10 -0400 Subject: [PATCH 4/4] revert(pycg): restore re-splitting of capped shards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts 98ecf05 ("fix(pycg): keep capped shards instead of re-splitting them"). Its supporting measurement was confounded: the whole-shard run completed three fixpoint passes while the halved run was cut off by the measurement harness after one, so the comparison spanned different analysis depths. A depth-matched control favored re-splitting. The whole series also ran with an unpinned hash seed — _pin_hash_seed() re-execs only when argv0 is the CLI, and the probes imported the library — so none of its absolute numbers are reliable. Without evidence the change does not stand: a capped shard is re-partitioned again, and only a shard that cannot be split further keeps its partial edges. Also rewrites the #145 changelog entry: drops seed-tainted timing figures, removes the claim that --pycg-max-iter guarantees termination, and links #148 (deterministic Jedi-only fallback for expensive shards). --- CHANGELOG.md | 46 ++++------ .../semantic_analysis/pycg/pycg_analysis.py | 92 +++++++------------ test/test_pycg_shard_determinism.py | 33 ++----- 3 files changed, 56 insertions(+), 115 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9154dd..0fd4321 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,30 +9,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **Exhausting `--pycg-max-iter` is no longer treated as a runaway** (#145). - Hitting the cap means PyCG returned a sound under-approximation — re-splitting - such a shard makes the answer *worse*, because every cut severs the calls - crossing it. Measured on one 100-file shard: bounding the fixpoint and keeping - the shard whole gave **110,490 edges**, where budget-driven halving of the same - shard gave **15,468**. (Edge counts are deterministic; wall-clock on this - workload is not — the same shard has taken 8 minutes and >81 minutes on the - same machine — so no timings are quoted.) Re-splitting also pays extra rounds - of re-analysis — - with a low `--pycg-max-iter` every shard hits the cap, and one such run on a - 2,364-file project took **2h50m without finishing**. A capped shard now - contributes its edges directly; only a shard that *raised* is decomposed. Both - classifications remain pure functions of the input, so reproducibility is - unaffected. - - **BREAKING: `--pycg-shard-timeout` is removed** (#145). It bounded PyCG's fixpoint a second time, by the clock, after `--pycg-max-iter` had already bounded it by iteration count — and that second bound is what made the output - load-dependent. PyCG terminates on its own at `--pycg-max-iter` (default 50), - so nothing is left unbounded at the default. Anyone passing - `--pycg-shard-timeout` must drop the flag; use `--pycg-max-iter` to trade - analysis depth against runtime. One caveat: `--pycg-max-iter -1` asks PyCG to - run to convergence with no cap, and there is no longer a wall-clock net behind - it, so a divergent shard can run indefinitely under that setting. + load-dependent. Anyone passing `--pycg-shard-timeout` must drop the flag; + `--pycg-max-iter` remains the knob that trades analysis depth against + runtime. Note that nothing bounds wall-clock time anymore: `--pycg-max-iter` + caps fixpoint *passes*, not their duration (see Fixed below), so a + pathological shard can still run long at any setting, and + `--pycg-max-iter -1` (run to convergence, no cap) can run indefinitely. - **BREAKING: the msgpack output format is removed** (#118, TS parity): the `--format msgpack` CLI choice, the `analysis.msgpack` artifact, the msgpack serialization mixin on schema models, and the `msgpack` dependency are gone. @@ -70,14 +55,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 alone. **`--pycg-max-iter` is not a termination guarantee.** It bounds fixpoint - *iterations* and is only consulted at pass boundaries, so a single - pathological pass escapes it — one shard ran >81 minutes at `max_iter=3` - without completing, while the identical shard completed in ~8 minutes on - another run. Removing the wall-clock timeout therefore removes the only - wall-clock bound that existed; that bound was non-deterministic and had to go, - but nothing replaces it yet. Adaptive decomposition is unchanged — a runaway is still re-partitioned - at a tighter budget to recover recall — but a shard that cannot be split - further now keeps the edges it did produce instead of being discarded. A + *passes*, and PyCG consults the cap only between passes, so one expensive + pass escapes it — a single 100-file shard has run for over an hour inside + its pass budget without completing. Removing the wall-clock timeout + therefore removes the only wall-clock bound that existed; that bound was + load-dependent and had to go, but nothing replaces it yet — #148 tracks + a deterministic fallback that routes predicted-expensive shards to + Jedi-only coverage. + + Adaptive decomposition is unchanged: a runaway is still re-partitioned at a + tighter file budget to recover recall, and a shard that cannot be split + further now keeps the edges it did produce instead of being discarded — a capped fixpoint is a sound under-approximation, so those edges are real. **Scope:** this removes the load-dependent shard-dropping mechanism, which was diff --git a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py index 4cfc149..90c115b 100644 --- a/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py +++ b/codeanalyzer/semantic_analysis/pycg/pycg_analysis.py @@ -473,12 +473,8 @@ class PyCG: # comes, takes many passes. A finite cap turns "loop until killed" into a # sound-but-incomplete result that still returns the edges found so far. # 50 is generous — well-behaved code converges in well under 20 passes — - # while bounding the pathological case. Note that lowering it does NOT - # reliably bound runtime: per-pass cost dominates, and a low cap makes - # nearly every shard hit it (measured: a 26-file shard needs 5 passes and - # yields the identical 93 edges at 3 and at 50). Override via - # --pycg-max-iter; -1 restores PyCG's unbounded run-to-convergence - # behaviour, with no wall-clock net behind it. + # while bounding the pathological case. Override via --pycg-max-iter; + # -1 restores PyCG's unbounded run-to-convergence behaviour. _PYCG_MAX_ITER: int = 50 # Iterative decomposition of runaway shards: a shard whose fixpoint stopped @@ -694,23 +690,17 @@ def _build_sharded_planned( PyCG's fixpoint diverges on heavy metaclass/mixin clusters, and a uniform ceiling would force *every* shard small (severing many edges) just to tame the few that run away. Instead we start coarse (low cut, high recall on - healthy code) and **only re-decompose the shards that FAILED**: a shard - that raised has its files re-partitioned at half the budget and re-run, - down to a floor. - - Exhausting ``--pycg-max-iter`` is deliberately NOT a runaway, because - splitting such a shard makes the result *worse*: every cut severs the - calls crossing it. Measured on one 100-file shard, bounding the - fixpoint and keeping the shard whole gave 110,490 edges, where - budget-driven halving of the same shard gave 15,468. (Edge counts are - deterministic; wall-clock on this workload is not -- the same shard has - taken 8 minutes and >81 minutes on the same machine, so timings are - deliberately not quoted.) Re-splitting also costs - whole extra rounds of re-analysis — with a low ``--pycg-max-iter`` every - shard hits the cap, and one such run took 2h50m without finishing. - - Runaway classification stays a function of the input, so the - reproducibility the wall-clock timeout cost us is preserved (#145). + healthy code) and **only re-decompose the shards that did not converge**: + each runaway's files are re-partitioned at half the budget and re-run, + down to a floor. A smaller shard has a smaller fixpoint to reach, so + splitting recovers the edges the capped pass missed while paying cut on + its internal seams alone. + + A shard is a runaway when its fixpoint stopped at ``--pycg-max-iter`` + rather than converging — a function of the input, so the same project + decomposes the same way every run. This used to be a wall-clock + timeout, which made *which* shards were dropped depend on machine load + and Ray scheduling (#145). The residue that still diverges at the floor (or is an atomic cycle that won't split) keeps the edges its capped fixpoint produced: a truncated @@ -736,7 +726,7 @@ def _build_sharded_planned( all_edges: List[PyCallEdge] = [] shards = plan.shards budget = self.shard_ceiling - accepted_total = 0 + converged_total = 0 irreducible_files = 0 round_no = 0 @@ -747,7 +737,7 @@ def _build_sharded_planned( logger.info("PyCG: %s", label) edges, runaways = runner(shards) all_edges.extend(edges) - accepted_total += len(shards) - len(runaways) + converged_total += len(shards) - len(runaways) if not runaways: break @@ -795,9 +785,9 @@ def _build_sharded_planned( result = self._coalesce_edges(all_edges) logger.info( - "PyCG: %d edges from %d accepted shard(s) over %d round(s) " + "PyCG: %d edges from %d converged shard(s) over %d round(s) " "(%d before dedup, Jedi-planned%s)", - len(result), accepted_total, round_no + 1, len(all_edges), + len(result), converged_total, round_no + 1, len(all_edges), ", Ray-parallel" if self.using_ray else "", ) return result @@ -807,22 +797,14 @@ def _run_fileset_shards_seq( ) -> Tuple[List[PyCallEdge], List[Tuple[List[str], List[PyCallEdge]]]]: """Run each file-set shard sequentially; return ``(edges, runaways)``. - A shard is a *runaway* only when it RAISED. Exhausting ``max_iter`` is - NOT a runaway: it means PyCG returned a sound under-approximation, and - re-splitting that shard makes the answer worse rather than better. - - Every cut severs the calls crossing it, so a split shard sees strictly - less. Measured on one 100-file shard: bounding the fixpoint and keeping - the shard whole gave 110,490 edges, where budget-driven halving of the - same shard gave 15,468. (Edge counts are deterministic; wall-clock is - not -- the same shard has taken 8 minutes and >81 minutes on the same - machine.) Re-splitting also pays whole extra rounds of - re-analysis -- with a low ``--pycg-max-iter`` every shard hits the cap, - and one such run took 2h50m without finishing. - - Both classifications remain pure functions of the input, so the - reproducibility this replaced the wall-clock timeout for is unaffected - (#145). + A shard is a *runaway* when PyCG stopped at ``max_iter`` instead of + reaching its fixpoint, or when it raised. Both are deterministic + functions of the input -- unlike the wall-clock timeout this replaced, + which made the surviving edge set depend on machine load (#145). + + Each runaway carries the edges it *did* produce. A capped fixpoint is a + sound under-approximation, so if decomposition cannot split the shard + further the caller keeps that partial rather than discarding it. """ resolver = self._resolver edges_all: List[PyCallEdge] = [] @@ -834,14 +816,10 @@ def _run_fileset_shards_seq( edges, converged = self._run_pycg_batch( eps, root, resolver, prefix="" ) - # Converged or capped, the edges are sound -- keep them. - edges_all.extend(edges) - if not converged: - logger.debug( - "PyCG shard: fixpoint capped at max_iter=%d " - "(expected; edges are a sound under-approximation)", - self.max_iter, - ) + if converged: + edges_all.extend(edges) + else: + runaways.append((files, edges)) except PyCGExceptions.PyCGAnalysisError: runaways.append((files, [])) progress.advance() @@ -907,14 +885,10 @@ def _run_fileset_shards_ray( PyCallEdge(src=s, dst=t, weight=w, prov=["pycg"]) for s, t, w in triples ] - # Converged or capped, the edges are sound -- keep them. - edges_all.extend(edges) - if not converged: - logger.debug( - "PyCG shard: fixpoint capped at max_iter=%d " - "(expected; sound under-approximation)", - self.max_iter, - ) + if converged: + edges_all.extend(edges) + else: + runaways.append((meta[fut], edges)) except Exception: runaways.append((meta[fut], [])) progress.advance() diff --git a/test/test_pycg_shard_determinism.py b/test/test_pycg_shard_determinism.py index f55542b..33d8650 100644 --- a/test/test_pycg_shard_determinism.py +++ b/test/test_pycg_shard_determinism.py @@ -53,38 +53,17 @@ def test_module_has_no_wall_clock_bound_left(): def test_sequential_runner_does_not_bound_shards_by_time(): src = _code_only(pa.PyCG._run_fileset_shards_seq) assert "_shard_timeout" not in src, "shard runs must not be wall-clock bounded" - assert "max_iter" in src, "the cap must be documented as expected, not a runaway" + assert "converged" in src, "runaway classification must use PyCG convergence" -def test_capped_shards_are_kept_not_re_split(): - """Exhausting max_iter is the expected case, not a runaway. +def test_runaways_carry_their_partial_edges(): + """A capped fixpoint is a sound under-approximation — keep it, don't drop it.""" + src = inspect.getsource(pa.PyCG._run_fileset_shards_seq) + assert "runaways.append((files, edges))" in src - --pycg-max-iter is a cost bound with a low default, so nearly every shard - hits it. Treating that as a runaway re-analysed the whole project up to - _PYCG_MAX_DECOMP_ROUNDS times (one measured run: 2h50m, unfinished), and - splitting destroys edges besides -- 110,490 edges in 95s keeping a shard - whole, versus 15,468 in 600s under budget-driven halving. - """ - for fn in (pa.PyCG._run_fileset_shards_seq, pa.PyCG._run_fileset_shards_ray): - src = _code_only(fn) - assert "runaways.append((files, edges))" not in src, ( - f"{fn.__name__}: a capped shard must not become a runaway" - ) - assert "runaways.append((meta[fut], edges))" not in src, ( - f"{fn.__name__}: a capped shard must not become a runaway" - ) - assert "edges_all.extend(edges)" in src, ( - f"{fn.__name__}: capped edges must still be kept" - ) - - -def test_failed_shards_are_still_re_split(): - """A shard that RAISED is still worth decomposing.""" - seq = _code_only(pa.PyCG._run_fileset_shards_seq) - assert "runaways.append((files, []))" in seq loop = inspect.getsource(pa.PyCG._build_sharded_planned) assert "all_edges.extend(partial)" in loop, ( - "an irreducible shard must contribute whatever edges it produced, not zero" + "an irreducible shard must contribute its capped-fixpoint edges, not zero" )