From 9129658647d56dea6a26d9fa20a5e64feac65229 Mon Sep 17 00:00:00 2001 From: Justyna Wojtczak Date: Mon, 4 May 2026 19:22:56 +0200 Subject: [PATCH] feat(steal): framework-aware code indexing for Steal precision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steal previously walked every tracked file in every repo, so a Rails app's `db/migrate/2024_billing.rb` ranked alongside `app/models/billing.rb` on a "billing" query — match-quality became a noise problem rather than a ranking problem. `code_block_service` now consults a per-repo profile detected from the manifest (Gemfile, pyproject, package.json). The profile decides which directories feed the 40-line window machinery: app/ + lib/ for Rails, src/ for src-layout Python, etc. Excludes (`migrations/`, `db/migrate/`) match anywhere in the path so they catch nested Django apps too. Repos with no recognisable manifest fall back to `unknown` and keep the previous "everything tracked" behaviour, so markdown- or shell-heavy projects index normally. Side benefits wired in alongside: - `steal_no_results` table tracks zero-hit queries so over-pruning surfaces in days, not months - diagnostic `armillary scan --report-profiles` aggregates the breakdown - code_index SCHEMA_VERSION bump → drop-and-rebuild on first scan after upgrade - per-repo (profile, files_indexed, files_skipped) recorded to metadata_json, observable when a custom layout indexes nothing Co-Authored-By: Claude Opus 4.7 (1M context) --- src/armillary/cache_mapping.py | 8 + src/armillary/cli.py | 87 ++++++++ src/armillary/code_block_service.py | 68 +++++- src/armillary/code_index.py | 2 +- src/armillary/feedback_service.py | 50 +++++ src/armillary/framework_profiles.py | 315 ++++++++++++++++++++++++++++ src/armillary/models.py | 8 + src/armillary/scan_service.py | 47 ++++- src/armillary/steal_service.py | 8 + tests/test_code_block_service.py | 71 +++++++ tests/test_feedback_service.py | 32 ++- tests/test_framework_profiles.py | 192 +++++++++++++++++ 12 files changed, 878 insertions(+), 10 deletions(-) create mode 100644 src/armillary/framework_profiles.py create mode 100644 tests/test_framework_profiles.py diff --git a/src/armillary/cache_mapping.py b/src/armillary/cache_mapping.py index 3d18216..f19ee0b 100644 --- a/src/armillary/cache_mapping.py +++ b/src/armillary/cache_mapping.py @@ -116,6 +116,10 @@ def _row_to_metadata(row: sqlite3.Row) -> ProjectMetadata | None: monthly_commits=extra.get("monthly_commits"), branch_count=extra.get("branch_count"), has_remote=extra.get("has_remote"), + # ADR 0031 — framework-aware indexing observability. + index_profile=extra.get("index_profile"), + index_files_indexed=extra.get("index_files_indexed"), + index_files_skipped=extra.get("index_files_skipped"), status=_safe_status(row["status"]), ) @@ -142,6 +146,10 @@ def _serialize_metadata_extra(md: ProjectMetadata) -> str | None: "monthly_commits": md.monthly_commits, "branch_count": md.branch_count, "has_remote": md.has_remote, + # ADR 0031. + "index_profile": md.index_profile, + "index_files_indexed": md.index_files_indexed, + "index_files_skipped": md.index_files_skipped, } # Drop empty keys to keep the JSON small and the diff readable. cleaned = {k: v for k, v in payload.items() if v not in (None, [], "")} diff --git a/src/armillary/cli.py b/src/armillary/cli.py index 2dd23ab..530ae26 100644 --- a/src/armillary/cli.py +++ b/src/armillary/cli.py @@ -209,6 +209,16 @@ def scan( "fresh project table. No-op if ~/.claude/ does not exist." ), ), + report_profiles: bool = typer.Option( + False, + "--report-profiles", + help=( + "Diagnostic for ADR 0031: after the scan, print how many " + "repos were classified into each framework profile. Helps " + "spot when profile detection misses a layout (e.g. lots of " + "repos falling into 'unknown')." + ), + ), ) -> None: """Scan umbrella folders and print the project list as JSON. @@ -236,6 +246,31 @@ def scan( ) raise typer.Exit(2) + if no_cache and report_profiles: + # Same shape of conflict: --report-profiles reads back from the + # cache (the indexer only runs on the cache code path). Skipping + # the cache means there is nothing to report on. + typer.secho( + "--report-profiles cannot be combined with --no-cache — the " + "report reads from the cache. Drop one flag.", + fg=typer.colors.RED, + err=True, + ) + raise typer.Exit(2) + + if no_metadata and report_profiles: + # `_index_code_blocks` skips projects whose `metadata` is None, + # which is the case for every project when `--no-metadata` is + # set. The report would always be empty / stale — better to + # reject the combination than print nothing. + typer.secho( + "--report-profiles cannot be combined with --no-metadata — " + "the indexer needs project metadata to run. Drop one flag.", + fg=typer.colors.RED, + err=True, + ) + raise typer.Exit(2) + umbrellas = _resolve_umbrellas(umbrella, max_depth) if not umbrellas: typer.secho( @@ -257,6 +292,11 @@ def scan( payload = [p.model_dump(mode="json") for p in projects] typer.echo(json.dumps(payload, indent=2, ensure_ascii=False)) + if report_profiles: + with Cache() as cache: + cached_projects = cache.list_projects() + _print_profile_report(cached_projects) + if refresh_bridge: claude_dir = Path.home() / ".claude" if not claude_dir.is_dir(): @@ -284,6 +324,53 @@ def scan( ) +def _print_profile_report(projects: list) -> None: + """Aggregate ADR 0031 indexing stats from a project list. + + Prints a Rich table with one row per detected profile + totals: + repo count, total files indexed, total files filtered out by the + profile. Only repos that ran through the Steal indexer (have + ``index_profile`` set on metadata) contribute. ``unknown`` is its + own row so a high count is immediately visible. + """ + from collections import defaultdict + + counts: dict[str, dict[str, int]] = defaultdict( + lambda: {"repos": 0, "indexed": 0, "skipped": 0} + ) + for p in projects: + md = p.metadata + if md is None or md.index_profile is None: + continue + bucket = counts[md.index_profile] + bucket["repos"] += 1 + bucket["indexed"] += md.index_files_indexed or 0 + bucket["skipped"] += md.index_files_skipped or 0 + + if not counts: + typer.secho( + "--report-profiles: no indexed projects in this scan.", + fg=typer.colors.YELLOW, + err=True, + ) + return + + table = Table(title="Framework profile breakdown (ADR 0031)", show_lines=False) + table.add_column("Profile", style="cyan", no_wrap=True) + table.add_column("Repos", justify="right") + table.add_column("Files indexed", justify="right") + table.add_column("Files filtered", justify="right") + for name in sorted(counts): + c = counts[name] + table.add_row( + name, + str(c["repos"]), + str(c["indexed"]), + str(c["skipped"]), + ) + Console(stderr=True).print(table) + + @app.command("list") def list_projects( type_filter: ProjectType | None = typer.Option( diff --git a/src/armillary/code_block_service.py b/src/armillary/code_block_service.py index f68dd49..ff55948 100644 --- a/src/armillary/code_block_service.py +++ b/src/armillary/code_block_service.py @@ -17,6 +17,8 @@ from dataclasses import dataclass from pathlib import Path +from .framework_profiles import Profile + _WINDOW_SIZE = 40 _WINDOW_STRIDE = 20 _MAX_FILE_BYTES = 500 * 1024 @@ -86,6 +88,21 @@ ) +@dataclass(frozen=True) +class RepoIndexResult: + """Profile + counts after walking a repo (ADR 0031 observability). + + Returned alongside the block list so :mod:`scan_service` can + persist ``files_indexed`` / ``files_skipped`` on the project + metadata blob — that's the regression guard against a profile + that detects a custom Rails layout and silently indexes nothing. + """ + + profile_name: str + files_indexed: int + files_skipped: int + + @dataclass(frozen=True) class CodeBlock: """A 40-line window from a source file, plus a heuristic symbol. @@ -106,22 +123,55 @@ class CodeBlock: updated_at: float -def build_blocks_for_repo(repo_path: Path) -> list[CodeBlock]: - """Return every 40-line window across all tracked files in a repo. +def build_blocks_for_repo( + repo_path: Path, + *, + profile: Profile | None = None, +) -> list[CodeBlock]: + """Return every 40-line window across tracked files in a repo. + + When ``profile`` is set (ADR 0031), only files whose path is + accepted by ``profile.accepts(rel)`` reach window extraction — + everything else (migrations, generated assets, fixtures) is + filtered out before it can dilute Steal's index. ``profile=None`` + keeps ADR 0027's original "everything tracked" behaviour and is + the default so existing callers don't change. Non-git directories (no ``.git``) and files matching the skip lists are filtered out. Size-capped at 500 KB per file; binaries are detected by null-byte sniff on the first 8 KB. Errors on individual files never escape — that file is skipped and the walk continues. """ + blocks, _ = build_blocks_with_stats(repo_path, profile=profile) + return blocks + + +def build_blocks_with_stats( + repo_path: Path, + *, + profile: Profile | None = None, +) -> tuple[list[CodeBlock], RepoIndexResult]: + """Same as :func:`build_blocks_for_repo` but also return profile stats. + + ``files_indexed`` counts files that produced at least one block; + ``files_skipped`` counts files filtered out by the profile (does + not include files dropped by the per-file skip rules — those are + universal and not profile-specific). + """ + profile_name = profile.name if profile is not None else "unknown" if not (repo_path / ".git").exists(): - return [] + return [], RepoIndexResult(profile_name, 0, 0) tracked = _git_ls_files(repo_path) blocks: list[CodeBlock] = [] repo_str = str(repo_path) + files_indexed = 0 + files_skipped_by_profile = 0 for rel in tracked: + if profile is not None and not profile.accepts(rel): + files_skipped_by_profile += 1 + continue abs_path = repo_path / rel if _should_skip(rel, abs_path): continue @@ -129,9 +179,15 @@ def build_blocks_for_repo(repo_path: Path) -> list[CodeBlock]: file_blocks = _blocks_for_file(repo_str, abs_path) except (OSError, UnicodeDecodeError): continue - blocks.extend(file_blocks) - - return blocks + if file_blocks: + files_indexed += 1 + blocks.extend(file_blocks) + + return blocks, RepoIndexResult( + profile_name=profile_name, + files_indexed=files_indexed, + files_skipped=files_skipped_by_profile, + ) # ----- git listing ---------------------------------------------------------- diff --git a/src/armillary/code_index.py b/src/armillary/code_index.py index 0eb18b3..6f842d7 100644 --- a/src/armillary/code_index.py +++ b/src/armillary/code_index.py @@ -23,7 +23,7 @@ from .cache import default_db_path as _default_project_db_path -SCHEMA_VERSION = 1 +SCHEMA_VERSION = 2 # ADR 0031: force rebuild so old "everything" indexes are dropped. _SCHEMA_SQL = """ CREATE TABLE code_blocks_meta ( diff --git a/src/armillary/feedback_service.py b/src/armillary/feedback_service.py index 17e4a99..39c910f 100644 --- a/src/armillary/feedback_service.py +++ b/src/armillary/feedback_service.py @@ -30,6 +30,21 @@ ); CREATE INDEX IF NOT EXISTS idx_steal_feedback_hash ON steal_feedback(query_hash); + +-- ADR 0031: continuous precision signal. A row per query that came +-- back empty so we can spot profiles over-pruning before users +-- complain. `path` and `start_line` mirror `steal_feedback`'s shape +-- but are unused for no-result events (kept NULL-equivalent: empty +-- string and 0) — schema parity buys nothing here, a sibling table +-- buys clarity. +CREATE TABLE IF NOT EXISTS steal_no_results ( + id INTEGER PRIMARY KEY, + query_hash TEXT NOT NULL, + query TEXT NOT NULL, + seen_at REAL NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_steal_no_results_hash + ON steal_no_results(query_hash); """ @@ -82,6 +97,41 @@ def vote_counts(query: str) -> dict[str, int]: return out +def record_no_results(query: str) -> None: + """Log that ``query`` returned no Steal hits (ADR 0031 signal). + + Cheap append. Aggregated by :func:`no_results_count` so we can + detect profiles over-pruning a query class — Arvid's continuous + feedback loop instead of a one-shot precision benchmark. + """ + normalised = query.strip() + if not normalised: + return + query_hash = _hash_query(normalised) + with CodeIndex() as idx: + _ensure_feedback_table(idx.conn) + idx.conn.execute( + "INSERT INTO steal_no_results (query_hash, query, seen_at) VALUES (?,?,?)", + (query_hash, normalised, time.time()), + ) + idx.conn.commit() + + +def no_results_count(query: str) -> int: + """Return how many times ``query`` has been logged as zero-hit.""" + normalised = query.strip() + if not normalised: + return 0 + query_hash = _hash_query(normalised) + with CodeIndex() as idx: + _ensure_feedback_table(idx.conn) + row = idx.conn.execute( + "SELECT COUNT(*) FROM steal_no_results WHERE query_hash = ?", + (query_hash,), + ).fetchone() + return int(row[0]) if row else 0 + + def _hash_query(query: str) -> str: return hashlib.sha1(query.lower().encode("utf-8")).hexdigest() # noqa: S324 diff --git a/src/armillary/framework_profiles.py b/src/armillary/framework_profiles.py new file mode 100644 index 0000000..9541608 --- /dev/null +++ b/src/armillary/framework_profiles.py @@ -0,0 +1,315 @@ +"""Framework-aware code-indexing profiles (ADR 0031). + +Detects what kind of project a repo is — Rails, Django, Next, plain +Python lib, etc. — from its manifest files, and returns a ``Profile`` +that tells :func:`code_block_service.build_blocks_for_repo` which +directories to feed into the 40-line window machinery. + +The fallback profile is ``unknown`` (``include`` = ``None``), which +preserves ADR 0027's original "index everything tracked" behaviour +for repos with no recognisable manifest. Profiles are plain data — +adding a new one means a new entry in :data:`_PROFILES` plus a +detection branch in :func:`detect_profile`. No plugin registry, no +YAML, no runtime config. + +Public API: :func:`detect_profile`, :class:`Profile`, :func:`UNKNOWN`. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from pathlib import Path + + +@dataclass(frozen=True) +class Profile: + """Indexing profile for a repo: which dirs are code, which are noise. + + ``include`` is a whitelist of path prefixes (relative to repo root, + POSIX-style, no leading slash). When ``None`` the indexer walks the + whole tree (current ``unknown`` behaviour). When set, only files + whose path starts with one of these prefixes are indexed. + + ``exclude`` always wins over ``include`` — useful when a profile + whitelists ``app/`` but wants to skip ``app/assets/builds/``. + """ + + name: str + include: tuple[str, ...] | None + exclude: tuple[str, ...] = field(default_factory=tuple) + + def accepts(self, rel_path: str) -> bool: + """True if ``rel_path`` (POSIX, no leading slash) survives filtering. + + Includes are anchored to the repo root — ``app/`` whitelists + ``app/...`` but not ``vendor/app/...``, because we want a + Rails-shaped layout, not just any directory called "app". + + Excludes are matched **anywhere** in the path: ``migrations/`` + excludes a Django migrations folder whether it lives at the + root (Django flat-layout) or under ``src/myapp/migrations/`` + (src-layout). The single trip through this matcher per file + is fine; we are post-`git ls-files` already. + """ + normalised = rel_path.lstrip("/") + for ex in self.exclude: + if _matches_anywhere(normalised, ex): + return False + if self.include is None: + return True + return any(_matches_prefix(normalised, inc) for inc in self.include) + + +def _matches_prefix(rel_path: str, prefix: str) -> bool: + """Match ``rel_path`` against a root-anchored directory prefix. + + A prefix ending with ``/`` matches any path *under* that directory; + a prefix without ``/`` matches that exact path or any descendant + (``lib`` matches ``lib/foo.rb`` but not ``library.rb``). + """ + norm_prefix = prefix.rstrip("/") + if not norm_prefix: + return True + return rel_path == norm_prefix or rel_path.startswith(norm_prefix + "/") + + +def _matches_anywhere(rel_path: str, pattern: str) -> bool: + """Match ``pattern`` as a contiguous run of segments anywhere in the path. + + ``migrations/`` matches ``foo/migrations/x.py`` and ``migrations/x.py``, + but not ``my_migrations/`` (segment boundary required). + ``app/assets/builds/`` matches only the contiguous run, so a stray + ``app/`` deep in the tree does not accidentally pull in everything. + """ + norm = pattern.rstrip("/") + if not norm: + return True + if _matches_prefix(rel_path, norm): + return True + needle = "/" + norm + "/" + if needle in rel_path: + return True + return rel_path.endswith("/" + norm) + + +# ----- profiles ------------------------------------------------------------- + + +UNKNOWN = Profile(name="unknown", include=None, exclude=()) + +_PROFILES: dict[str, Profile] = { + "rails": Profile( + name="rails", + include=( + "app/", + "lib/", + "spec/integration/", + "spec/system/", + "spec/features/", + "test/integration/", + "test/system/", + ), + exclude=( + "app/assets/builds/", + "app/assets/images/", + ), + ), + "ruby_gem": Profile( + name="ruby_gem", + include=( + "lib/", + "spec/integration/", + "spec/system/", + "test/integration/", + ), + exclude=(), + ), + "python_web": Profile( + name="python_web", + include=( + "src/", + "tests/integration/", + "tests/e2e/", + ), + exclude=( + "migrations/", + "static/", + "media/", + ), + ), + "python_lib": Profile( + name="python_lib", + include=( + "src/", + "tests/integration/", + "tests/e2e/", + ), + exclude=(), + ), + "js_frontend": Profile( + name="js_frontend", + include=( + "src/", + "app/", + "pages/", + "components/", + ), + exclude=( + "public/", + ".next/", + "out/", + ), + ), + "node": Profile( + name="node", + include=( + "src/", + "lib/", + "bin/", + ), + exclude=( + "dist/", + "coverage/", + ), + ), +} + + +# ----- detection ------------------------------------------------------------ + + +def detect_profile(repo_path: Path) -> Profile: + """Pick the right :class:`Profile` for a repo by inspecting manifests. + + Detection order matters: Rails before plain Ruby (a Rails app has + a Gemfile too); framework-specific Python before plain Python; JS + frontend before plain Node. Anything we can't classify, *or* a + manifest we can't read, falls back to :data:`UNKNOWN` so the + indexer keeps current behaviour rather than silently classifying + a torn file as the negative branch. + """ + try: + gemfile = repo_path / "Gemfile" + if gemfile.is_file(): + content = _safe_read(gemfile) + if content is None: + return UNKNOWN + if _gemfile_mentions_rails(content): + return _PROFILES["rails"] + return _PROFILES["ruby_gem"] + + pyproject = repo_path / "pyproject.toml" + if pyproject.is_file(): + content = _safe_read(pyproject) + if content is None: + return UNKNOWN + if _pyproject_mentions_web(content): + return _python_profile(repo_path, _PROFILES["python_web"]) + return _python_profile(repo_path, _PROFILES["python_lib"]) + + package_json = repo_path / "package.json" + if package_json.is_file(): + content = _safe_read(package_json) + if content is None: + return UNKNOWN + if _package_json_mentions_frontend(content): + return _PROFILES["js_frontend"] + return _PROFILES["node"] + except OSError: + return UNKNOWN + + return UNKNOWN + + +def _python_profile(repo_path: Path, base: Profile) -> Profile: + """Adjust a Python profile for src-layout vs flat-layout. + + The ``src/`` whitelist works for src-layout repos but indexes + nothing on flat-layout repos (where the package lives at + ``//``). When ``src/`` does not exist we fall back + to :data:`UNKNOWN` rather than apply a whitelist that would drop + everything — silently indexing zero files is worse than indexing + everything tracked. + """ + if (repo_path / "src").is_dir(): + return base + return UNKNOWN + + +# ----- manifest sniffers ---------------------------------------------------- + + +def _safe_read(path: Path) -> str | None: + """Read a manifest as text; ``None`` signals a torn read. + + A read failure (permission, race, encoding without ``replace`` — + we use ``replace`` so genuine encoding errors do not surface) is + distinguishable from an empty file; callers should treat ``None`` + as "I can't classify" and fall back to :data:`UNKNOWN`. Returning + ``""`` would silently take the negative branch of every sniffer + and misclassify the repo as ``ruby_gem`` / ``python_lib`` / ``node``. + """ + try: + return path.read_text(encoding="utf-8", errors="replace") + except OSError: + return None + + +def _gemfile_mentions_rails(content: str) -> bool: + """Match ``gem "rails"`` / ``gem 'rails'`` (any version pin).""" + lower = content.lower() + return ('gem "rails"' in lower) or ("gem 'rails'" in lower) + + +_PYTHON_WEB_DISTS: tuple[str, ...] = ("django", "fastapi", "flask", "starlette") +_JS_FRONTEND_DISTS: tuple[str, ...] = ("react", "next", "vue", "svelte", "nuxt") + + +def _pyproject_mentions_web(content: str) -> bool: + """Match a dependency on django / fastapi / flask / starlette. + + Covers both common spellings: + + * PEP 621 / PDM style — quoted strings like ``"django>=5.0"`` in + a ``dependencies`` array. + * Poetry style — unquoted keys like ``django = "^5.0"`` under + ``[tool.poetry.dependencies]``. + + For Poetry we look for `` =`` at the start of any line, so + a substring like ``packages = ["my_django_helpers"]`` cannot + misfire. Scope matches the single-quoted variants too (rare in + pyproject but cheap to support) and is case-insensitive. + """ + lower = content.lower() + if any(f'"{dist}' in lower for dist in _PYTHON_WEB_DISTS): + return True + return _has_poetry_dep(lower, _PYTHON_WEB_DISTS) + + +def _package_json_mentions_frontend(content: str) -> bool: + """Match react / next / vue / svelte / nuxt as deps. + + Same opening-quote prefix trick as :func:`_pyproject_mentions_web` + so ``"react": "^18"`` and ``"react-dom"`` both classify as + js_frontend. + """ + lower = content.lower() + return any(f'"{dist}' in lower for dist in _JS_FRONTEND_DISTS) + + +def _has_poetry_dep(lower_content: str, dists: tuple[str, ...]) -> bool: + """True if any `` =`` line appears (Poetry / PDM legacy syntax). + + Inspects the content line-by-line to make the anchor explicit: + ``django = "^5.0"`` matches, ``# django = "^5.0"`` does not, and + a TOML key like ``packages = [...]`` cannot be mistaken for a + dependency. A single-quoted variant is also accepted. + """ + for raw_line in lower_content.splitlines(): + line = raw_line.lstrip() + if not line or line.startswith("#"): + continue + for dist in dists: + if line.startswith(f"{dist} =") or line.startswith(f"{dist}="): + return True + return False diff --git a/src/armillary/models.py b/src/armillary/models.py index 0549d6a..429762c 100644 --- a/src/armillary/models.py +++ b/src/armillary/models.py @@ -116,6 +116,14 @@ class ProjectMetadata(BaseModel): # the cache and dashboard can both read it as part of `ProjectMetadata`. status: Status | None = None + # ADR 0031 — framework-aware code indexing observability. + # Recorded by `scan_service._index_code_blocks` after a Steal index + # run so a custom layout that gets `index_files_indexed=0` is + # detectable from the cache instead of silently failing. + index_profile: str | None = None + index_files_indexed: int | None = None + index_files_skipped: int | None = None + class Project(BaseModel): """A single discovered project.""" diff --git a/src/armillary/scan_service.py b/src/armillary/scan_service.py index 02ea8b8..a47a215 100644 --- a/src/armillary/scan_service.py +++ b/src/armillary/scan_service.py @@ -78,18 +78,34 @@ def _index_code_blocks(projects: list[Project]) -> None: Isolated in its own helper so `contextlib.suppress` in `full_scan` catches the whole sub-pipeline (FTS5 unsupported, disk full, etc.) without swallowing more than we intend. + + Per ADR 0031: each repo is indexed under a framework profile + detected from its manifests. The profile name plus + files_indexed / files_skipped counts are persisted to the project + metadata blob so a misclassified layout is observable instead of + silent. """ import contextlib as _ctx - from .code_block_service import build_blocks_for_repo + from .cache import Cache + from .code_block_service import build_blocks_with_stats from .code_index import CodeIndex + from .framework_profiles import detect_profile + + stats_by_path: dict[str, tuple[str, int, int]] = {} with CodeIndex() as idx: for project in projects: if project.type is not ProjectType.GIT: continue with _ctx.suppress(Exception): - blocks = build_blocks_for_repo(project.path) + profile = detect_profile(project.path) + blocks, result = build_blocks_with_stats(project.path, profile=profile) + stats_by_path[str(project.path)] = ( + result.profile_name, + result.files_indexed, + result.files_skipped, + ) # Group by file path — upsert per-file so a partial # failure leaves earlier files indexed. by_file: dict[str, list] = {} @@ -99,6 +115,33 @@ def _index_code_blocks(projects: list[Project]) -> None: for path_str, file_blocks in by_file.items(): idx.upsert_blocks(str(project.path), path_str, file_blocks) + # Persist profile observability back to the project cache. We + # deliberately do NOT mutate `Project.metadata` on the in-memory + # list — `armillary scan` prints those objects as JSON and the + # contract (test_scan_json_output_unchanged_when_caching) is that + # stdout is invariant to whether the cache was written. So we + # build copies, upsert those, and let `--report-profiles` read + # from the cache after the scan completes. + if not stats_by_path: + return + touched: list[Project] = [] + for project in projects: + stats = stats_by_path.get(str(project.path)) + if stats is None or project.metadata is None: + continue + name, indexed, skipped = stats + md_copy = project.metadata.model_copy( + update={ + "index_profile": name, + "index_files_indexed": indexed, + "index_files_skipped": skipped, + } + ) + touched.append(project.model_copy(update={"metadata": md_copy})) + if touched: + with _ctx.suppress(Exception), Cache() as cache: + cache.upsert(touched, write_metadata=True) + def initial_scan(umbrellas: list[UmbrellaFolder]) -> list[Project]: """Walk + extract + status compute + cache clear + upsert (no prune). diff --git a/src/armillary/steal_service.py b/src/armillary/steal_service.py index 5bfe166..49ddacc 100644 --- a/src/armillary/steal_service.py +++ b/src/armillary/steal_service.py @@ -166,6 +166,14 @@ def steal( rows = idx.search(query, limit=overfetch, language_ext=language) if not rows: + # ADR 0031 — log zero-hit queries so we can spot profile defaults + # over-pruning. Best-effort: never break the steal call path. + import contextlib as _ctx + + from . import feedback_service as _fb + + with _ctx.suppress(Exception): + _fb.record_no_results(query) return [] # Diversify the re-ranking pool: keep at most N hits per repo so a diff --git a/tests/test_code_block_service.py b/tests/test_code_block_service.py index 3dc7816..b44dbd5 100644 --- a/tests/test_code_block_service.py +++ b/tests/test_code_block_service.py @@ -13,7 +13,9 @@ _extract_symbol, _should_skip, build_blocks_for_repo, + build_blocks_with_stats, ) +from armillary.framework_profiles import Profile def _init_git_repo(root: Path) -> None: @@ -171,3 +173,72 @@ def test_codeblock_is_frozen() -> None: ) with pytest.raises((AttributeError, TypeError, FrozenInstanceError)): blk.path = "/r/b.py" # type: ignore[misc] + + +# ----- ADR 0031 — framework profile filtering ------------------------------ + + +def _rails_layout_repo(root: Path) -> None: + """Set up a tiny git repo that mimics a Rails skeleton.""" + _init_git_repo(root) + (root / "Gemfile").write_text('gem "rails"\n') + (root / "app" / "models").mkdir(parents=True) + (root / "app" / "models" / "user.rb").write_text( + "class User\n def login\n :ok\n end\nend\n" + ) + (root / "db" / "migrate").mkdir(parents=True) + (root / "db" / "migrate" / "20240101_create_users.rb").write_text( + "class CreateUsers < ActiveRecord::Migration[7.0]\n def change\n end\nend\n" + ) + (root / "config").mkdir() + (root / "config" / "routes.rb").write_text( + "Rails.application.routes.draw do\nend\n" + ) + _git_add_commit(root) + + +def test_profile_drops_files_outside_include(tmp_path: Path) -> None: + _rails_layout_repo(tmp_path) + rails = Profile(name="rails", include=("app/", "lib/")) + + blocks = build_blocks_for_repo(tmp_path, profile=rails) + + rels = {Path(b.path).relative_to(tmp_path).as_posix() for b in blocks} + assert "app/models/user.rb" in rels + # db/migrate and config are filtered out by the profile. + assert not any(r.startswith("db/migrate/") for r in rels) + assert not any(r.startswith("config/") for r in rels) + + +def test_profile_none_keeps_legacy_index_everything_behaviour(tmp_path: Path) -> None: + _rails_layout_repo(tmp_path) + blocks = build_blocks_for_repo(tmp_path, profile=None) + rels = {Path(b.path).relative_to(tmp_path).as_posix() for b in blocks} + # No profile = same as ADR 0027 v1 — db/migrate IS indexed. + assert any(r.startswith("db/migrate/") for r in rels) + + +def test_profile_stats_record_indexed_and_skipped_counts(tmp_path: Path) -> None: + _rails_layout_repo(tmp_path) + rails = Profile(name="rails", include=("app/",)) + _, result = build_blocks_with_stats(tmp_path, profile=rails) + assert result.profile_name == "rails" + assert result.files_indexed >= 1 # at least app/models/user.rb + assert result.files_skipped >= 2 # Gemfile, db/migrate, config files + + +def test_profile_indexes_zero_files_observable(tmp_path: Path) -> None: + """Custom Rails layout with no app/ — observability regression guard.""" + _init_git_repo(tmp_path) + (tmp_path / "Gemfile").write_text('gem "rails"\n') + # No app/ — engine-style or unusual layout. + (tmp_path / "engines").mkdir() + (tmp_path / "engines" / "billing.rb").write_text("class Billing\nend\n") + _git_add_commit(tmp_path) + + rails = Profile(name="rails", include=("app/", "lib/")) + blocks, result = build_blocks_with_stats(tmp_path, profile=rails) + assert blocks == [] + assert result.files_indexed == 0 + # The Gemfile + engines/billing.rb were filtered → at least 2 skipped. + assert result.files_skipped >= 2 diff --git a/tests/test_feedback_service.py b/tests/test_feedback_service.py index 452e93f..a6031dc 100644 --- a/tests/test_feedback_service.py +++ b/tests/test_feedback_service.py @@ -6,7 +6,12 @@ import pytest -from armillary.feedback_service import record_vote, vote_counts +from armillary.feedback_service import ( + no_results_count, + record_no_results, + record_vote, + vote_counts, +) @pytest.fixture @@ -38,3 +43,28 @@ def test_query_normalisation_case_insensitive(isolated_index: Path) -> None: record_vote("Stripe Webhook", "/x.py", 1, 1) # Different casing and surrounding whitespace hits the same bucket. assert vote_counts(" stripe webhook ") == {"up": 1, "down": 0} + + +# ----- ADR 0031 — no-results signal ---------------------------------------- + + +def test_no_results_count_starts_at_zero(isolated_index: Path) -> None: + assert no_results_count("nothing here") == 0 + + +def test_record_no_results_accumulates(isolated_index: Path) -> None: + record_no_results("rails authn middleware") + record_no_results("rails authn middleware") + record_no_results("rails authn middleware") + assert no_results_count("rails authn middleware") == 3 + + +def test_no_results_query_normalised_like_votes(isolated_index: Path) -> None: + record_no_results("Stripe Webhook") + assert no_results_count(" stripe webhook ") == 1 + + +def test_empty_no_results_query_ignored(isolated_index: Path) -> None: + record_no_results("") + record_no_results(" ") + assert no_results_count("") == 0 diff --git a/tests/test_framework_profiles.py b/tests/test_framework_profiles.py new file mode 100644 index 0000000..b5ad7c3 --- /dev/null +++ b/tests/test_framework_profiles.py @@ -0,0 +1,192 @@ +"""Tests for framework_profiles — manifest detection + Profile.accepts.""" + +from __future__ import annotations + +from pathlib import Path + +from armillary.framework_profiles import UNKNOWN, Profile, detect_profile + +# ----- Profile.accepts ------------------------------------------------------ + + +def test_unknown_profile_accepts_everything() -> None: + assert UNKNOWN.accepts("anything/here.rb") + assert UNKNOWN.accepts("db/migrate/2024_init.rb") + + +def test_include_whitelist_keeps_only_listed_prefixes() -> None: + p = Profile(name="t", include=("app/", "lib/")) + assert p.accepts("app/models/user.rb") + assert p.accepts("lib/services/foo.rb") + assert not p.accepts("db/migrate/2024_init.rb") + assert not p.accepts("config/routes.rb") + + +def test_exclude_overrides_include() -> None: + p = Profile( + name="t", + include=("app/",), + exclude=("app/assets/builds/",), + ) + assert p.accepts("app/models/user.rb") + assert not p.accepts("app/assets/builds/foo.js") + + +def test_prefix_match_is_directory_aware() -> None: + """`lib` matches lib/ and lib/x.rb, not library.rb (avoid false positives).""" + p = Profile(name="t", include=("lib",)) + assert p.accepts("lib/x.rb") + assert p.accepts("lib") # exact + assert not p.accepts("library.rb") + assert not p.accepts("vendor/lib/x.rb") + + +# ----- detect_profile ------------------------------------------------------- + + +def test_detect_rails_from_gemfile_with_rails(tmp_path: Path) -> None: + (tmp_path / "Gemfile").write_text( + 'source "https://rubygems.org"\ngem "rails", "7.1"\n' + ) + profile = detect_profile(tmp_path) + assert profile.name == "rails" + assert "app/" in (profile.include or ()) + assert "spec/integration/" in (profile.include or ()) + + +def test_detect_rails_with_single_quoted_rails_gem(tmp_path: Path) -> None: + (tmp_path / "Gemfile").write_text("gem 'rails', '~> 7.0'\n") + assert detect_profile(tmp_path).name == "rails" + + +def test_detect_ruby_gem_from_gemfile_without_rails(tmp_path: Path) -> None: + (tmp_path / "Gemfile").write_text('gem "rspec"\ngem "thor"\n') + profile = detect_profile(tmp_path) + assert profile.name == "ruby_gem" + # ruby_gem must not include app/ — that's a Rails-specific dir. + assert "app/" not in (profile.include or ()) + + +def test_detect_python_web_from_pyproject_with_django(tmp_path: Path) -> None: + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "x"\ndependencies = ["django>=5.0"]\n' + ) + (tmp_path / "src").mkdir() + profile = detect_profile(tmp_path) + assert profile.name == "python_web" + assert "migrations/" in profile.exclude + + +def test_detect_python_lib_from_plain_pyproject(tmp_path: Path) -> None: + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "x"\ndependencies = ["pydantic>=2"]\n' + ) + (tmp_path / "src").mkdir() + assert detect_profile(tmp_path).name == "python_lib" + + +def test_detect_js_frontend_from_package_json_with_react(tmp_path: Path) -> None: + (tmp_path / "package.json").write_text( + '{"name":"x","dependencies":{"react":"^18.0.0"}}' + ) + assert detect_profile(tmp_path).name == "js_frontend" + + +def test_detect_node_from_plain_package_json(tmp_path: Path) -> None: + (tmp_path / "package.json").write_text( + '{"name":"x","dependencies":{"express":"^4"}}' + ) + assert detect_profile(tmp_path).name == "node" + + +def test_no_manifest_falls_back_to_unknown(tmp_path: Path) -> None: + (tmp_path / "README.md").write_text("# nothing here\n") + profile = detect_profile(tmp_path) + assert profile is UNKNOWN + assert profile.include is None + + +def test_rails_detection_wins_over_ruby_gem_when_both_signals_present( + tmp_path: Path, +) -> None: + """A Rails app technically has both Gemfile + rails dep — must classify as rails.""" + (tmp_path / "Gemfile").write_text('gem "rails"\ngem "rspec"\n') + assert detect_profile(tmp_path).name == "rails" + + +# ----- Codex review fixes --------------------------------------------------- + + +def test_exclude_matches_segment_anywhere_in_path() -> None: + """`migrations/` excludes Django migrations under src/myapp/, not just root.""" + p = Profile( + name="t", + include=("src/", "tests/integration/"), + exclude=("migrations/",), + ) + assert not p.accepts("src/myapp/migrations/0001_initial.py") + assert not p.accepts("migrations/0001_initial.py") + assert p.accepts("src/myapp/views.py") + # Segment boundary required — `my_migrations/` is not excluded. + assert p.accepts("src/my_migrations/foo.py") + + +def test_multi_segment_exclude_matches_contiguous_run() -> None: + p = Profile( + name="t", + include=("app/",), + exclude=("app/assets/builds/",), + ) + assert not p.accepts("app/assets/builds/manifest.js") + # A stray `app/` deep in the tree does not match — pattern is + # multi-segment, so contiguity matters. + assert p.accepts("app/models/user.rb") + + +def test_pyproject_poetry_syntax_classifies_as_python_web(tmp_path: Path) -> None: + """Poetry's `django = "^5.0"` form must classify as python_web (Codex #3).""" + (tmp_path / "pyproject.toml").write_text( + '[tool.poetry.dependencies]\npython = "^3.11"\ndjango = "^5.0"\n' + ) + (tmp_path / "src").mkdir() + assert detect_profile(tmp_path).name == "python_web" + + +def test_pyproject_poetry_does_not_misfire_on_unrelated_keys(tmp_path: Path) -> None: + """`packages = [...]` must NOT be mistaken for a dep on a flask-shaped key.""" + (tmp_path / "pyproject.toml").write_text( + "[tool.poetry]\n" + 'packages = ["my_flask_helpers"]\n' + "[tool.poetry.dependencies]\n" + 'requests = "^2"\n' + ) + (tmp_path / "src").mkdir() + assert detect_profile(tmp_path).name == "python_lib" + + +def test_python_flat_layout_falls_back_to_unknown(tmp_path: Path) -> None: + """Flat-layout repo (no src/) must classify as UNKNOWN, not python_lib (Codex #2). + + Otherwise the python_lib whitelist drops every source file in + a repo where the package lives at root. + """ + (tmp_path / "pyproject.toml").write_text( + '[project]\nname = "mypkg"\ndependencies = ["pydantic"]\n' + ) + (tmp_path / "mypkg").mkdir() + (tmp_path / "mypkg" / "__init__.py").write_text("") + profile = detect_profile(tmp_path) + assert profile.name == "unknown" + # Sanity: the flat package WOULD be indexed under unknown. + assert profile.accepts("mypkg/foo.py") + + +def test_manifest_unreadable_falls_back_to_unknown(tmp_path: Path, monkeypatch) -> None: + """A read failure on Gemfile must NOT silently classify as ruby_gem (Codex #5).""" + (tmp_path / "Gemfile").write_text('gem "rails"\n') + + def _broken(_self, encoding="utf-8", errors="replace"): # noqa: ARG001 + raise OSError("torn read") + + monkeypatch.setattr(Path, "read_text", _broken) + assert detect_profile(tmp_path).name == "unknown"