diff --git a/.gitignore b/.gitignore index fdbe9e3f..78a44107 100644 --- a/.gitignore +++ b/.gitignore @@ -569,6 +569,10 @@ coverage.xml **/.cache/decisions.jsonl.1 **/.cache/decisions.jsonl.lock +# doc-index cache (doc_index.py) — per-project structural index for JIT +# retrieval, read-once-per-file. Rebuilds automatically on content change. +**/.cache/doc-index/ + # Superpowers brainstorming/planning specs (local-only) docs/superpowers/ diff --git a/architecture/features/traceability-validation.md b/architecture/features/traceability-validation.md index b7494576..93c04949 100644 --- a/architecture/features/traceability-validation.md +++ b/architecture/features/traceability-validation.md @@ -21,6 +21,7 @@ - [List ID Kinds](#list-id-kinds) - [Validate TOC](#validate-toc) - [TOC Utilities](#toc-utilities) + - [Document Index](#document-index) - [Markdown Parsing Utilities](#markdown-parsing-utilities) - [Fixing Prompt Enrichment](#fixing-prompt-enrichment) - [Headings Contract Validation](#headings-contract-validation) @@ -398,11 +399,11 @@ Catches structural and traceability issues that AI agents miss or hallucinate 2. [x] - `p1` - Generate expected TOC from headings - `inst-toc-generate-expected` 3. [x] - `p1` - Compare existing vs expected: check anchor validity, heading coverage, staleness - `inst-toc-compare` 4. [x] - `p1` - **IF** mismatch, record error with diff details - `inst-toc-if-mismatch` -4. [x] - `p1` - **RETURN** JSON: `{status, files_checked, errors}` - `inst-toc-return` +4. [x] - `p1` - **RETURN** JSON: `{status, files_validated, error_count, warning_count, results}`, each `results[]` entry `{file, status, error_count, warning_count}` plus `errors`/`warnings` arrays when `--verbose` or non-empty - `inst-toc-return` **Supporting**: - [x] - `p1` - Imports and module setup for validate-toc command - `inst-toc-imports` -- [x] - `p1` - Human-friendly formatter for validate-toc output - `inst-toc-format` +- [x] - `p1` - Human-friendly formatter for validate-toc output: a WARN-only file prints its warnings the same way a FAIL file prints its errors, not just the bare status - `inst-toc-format` ### TOC Utilities @@ -416,6 +417,9 @@ Catches structural and traceability issues that AI agents miss or hallucinate 4. [x] - `p1` - Insert/update TOC using heading-based insertion (`## Table of Contents`) for kit file generator - `inst-toc-util-insert-heading` 5. [x] - `p1` - Process file: strip manual TOC, insert marker-based TOC, write if changed - `inst-toc-util-process-file` 6. [x] - `p1` - Validate TOC: check existence, anchor validity, completeness, staleness - `inst-toc-util-validate` +7. [x] - `p1` - Parse headings with line numbers, fence-aware and front-matter-aware (skips a leading YAML block so a `#`-prefixed front-matter line is never mistaken for a heading); shared by doc-index and the JIT-retrieval readiness checks, which need section boundaries the plain heading list doesn't carry -- `parse_headings` itself now delegates here, stripping the line number, so both share one fence/heading-match implementation - `inst-toc-util-parse-headings-lines` +8. [x] - `p1` - Collect JIT-retrieval readiness warnings for a document: gathers all four signals below over *every* heading level, independent of whatever level cap the caller configured for TOC-completeness checking - `inst-toc-jit-readiness-collect` +9. [x] - `p1` - Compute the four JIT-retrieval readiness signals -- duplicate heading titles (compared case-insensitively, with internal whitespace collapsed and Unicode-normalized, though the original text is still shown in the warning), heading depth jumps, oversized sections (`--max-section-lines`, default 300, validated against non-finite/non-positive input independent of the CLI's own argparse guard), and a missing top-of-file description/frontmatter block -- all warning-only, never errors (see constructorfabric/studio#104) - `inst-toc-jit-readiness` **Supporting**: - [x] - `p1` - Imports, constants, fence tracking, GitHub anchor slug generation - `inst-toc-util-datamodel` @@ -437,6 +441,35 @@ Catches structural and traceability issues that AI agents miss or hallucinate - [x] - `p1` - Heading-based TOC new-insert branch: inject `## Table of Contents` before first heading when absent - `inst-toc-util-insert-heading-new` - [x] - `p1` - TOC validate init: build heading list and expected TOC string before comparison checks - `inst-toc-util-validate-init` +### Document Index + +- [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-doc-index` + +**Input**: Markdown file path + +**Output**: `cfs doc-index`'s JSON is `{file, cache_hit, total_lines, section_count, sections}`, each `sections[]` entry `{level, heading, line_start, line_end, summary}`. The underlying index dict additionally carries `schema_version`, `path`, and `etag`. + +A cached, read-once-per-file structural index for Markdown JIT retrieval (see +constructorfabric/studio#104): parsing a file's headings/section boundaries +happens once, not once per query, until its stat fingerprint (`mtime_ns` + +size) changes. +The cache-validity fingerprint is deliberately metadata-only (`mtime` + file +size via `Path.stat()`), never a content hash — the point of the cache is to +avoid reading the file at all on a hit, and a content hash would defeat that +by requiring the read it's meant to save. + +1. [x] - `p1` - Build a fresh structural index: parse headings + line ranges from current content, compute the stat-based fingerprint, stamp the current schema version - `inst-doc-index-build` +2. [x] - `p1` - Load a cached index for a file, validated against current stat metadata (no content read on a hit) and against the required-field shape at the current schema version; returns `None` if missing, stale, corrupt, or an incomplete/outdated shape - `inst-doc-index-load` +3. [x] - `p1` - Persist an index to its cache location atomically (temp file + `os.replace`, so a concurrent reader never observes a torn write); no-ops silently outside a Studio-adapted project - `inst-doc-index-save` +4. [x] - `p1` - Return the cached index or build-and-cache a fresh one; reports cache hit/miss for benchmarking - `inst-doc-index-get-or-build` +5. [x] - `p1` - Attach a one-line, LLM-authored summary to a cached section by its `line_start`, for a future per-section-summary caller - `inst-doc-index-annotate` + +**Supporting**: +- [x] - `p1` - Stat-based cache-validity fingerprint (`mtime_ns` + size); resolved from the file's own path, never a content hash - `inst-doc-index-etag` +- [x] - `p1` - Resolve the cache file location within the Studio directory owning the indexed file, resolved from the file's own path (not the process's working directory) - `inst-doc-index-cache-path` +- [x] - `p1` - `cfs doc-index` CLI wrapper: parse arguments, build the JSON output payload, reporting a clean error for a missing or unreadable file - `inst-doc-index-cmd` +- [x] - `p1` - Human-friendly formatter for `cfs doc-index` output - `inst-doc-index-cmd-format` + ### Markdown Parsing Utilities - [x] `p1` - **ID**: `cpt-studio-algo-traceability-validation-parsing-utils` diff --git a/skills/studio/scripts/studio/cli.py b/skills/studio/scripts/studio/cli.py index dab43452..633509c2 100644 --- a/skills/studio/scripts/studio/cli.py +++ b/skills/studio/scripts/studio/cli.py @@ -134,6 +134,10 @@ def _cmd_eval(argv: List[str]) -> int: from .commands.eval import cmd_eval return cmd_eval(argv) +def _cmd_doc_index(argv: List[str]) -> int: + from .commands.doc_index import cmd_doc_index + return cmd_doc_index(argv) + # ============================================================================= # ADAPTER COMMAND # ============================================================================= @@ -216,6 +220,7 @@ def _cmd_map(argv: List[str]) -> int: "resolve-vars": "Resolve template variables to absolute paths", "toc": "Generate/update Table of Contents", "chunk-input": "Chunk oversized workflow input into line-bounded Markdown files", + "doc-index": "Build/reuse a cached heading index for a Markdown file (read once, not per query)", "pdsl": "Validate PDSL prompt blocks", "workspace-init": "Initialize multi-repo workspace", "workspace-add": "Add a source to workspace config", @@ -232,7 +237,7 @@ def _cmd_map(argv: List[str]) -> int: ("Validation", ["validate", "validate-kits", "validate-toc", "spec-coverage", "check-language"]), ("Search & Navigation", ["list-ids", "list-id-kinds", "get-content", "where-defined", "where-used"]), ("Kit Management", ["kit"]), - ("Utility", ["toc", "chunk-input", "pdsl"]), + ("Utility", ["toc", "chunk-input", "doc-index", "pdsl"]), ("Workspace", ["workspace-init", "workspace-add", "workspace-info", "workspace-sync"]), ("Delegation", ["delegate"]), ("Diagnostics", ["doctor"]), @@ -263,6 +268,7 @@ def _cmd_map(argv: List[str]) -> int: "validate-toc": "_cmd_validate_toc", "spec-coverage": "_cmd_spec_coverage", "chunk-input": "_cmd_chunk_input", + "doc-index": "_cmd_doc_index", "workspace-init": "_cmd_workspace_init", "workspace-add": "_cmd_workspace_add", "workspace-info": "_cmd_workspace_info", @@ -296,6 +302,7 @@ def _cmd_map(argv: List[str]) -> int: _cmd_validate_toc, _cmd_spec_coverage, _cmd_chunk_input, + _cmd_doc_index, _cmd_workspace_init, _cmd_workspace_add, _cmd_workspace_info, diff --git a/skills/studio/scripts/studio/commands/doc_index.py b/skills/studio/scripts/studio/commands/doc_index.py new file mode 100644 index 00000000..b71c493e --- /dev/null +++ b/skills/studio/scripts/studio/commands/doc_index.py @@ -0,0 +1,78 @@ +"""Studio doc-index command — build/reuse a cached structural index for a +Markdown file, so heading-based JIT retrieval reads a file's structure once, +not once per query. + +Thin CLI wrapper around ``studio.utils.doc_index``. +""" + +import argparse +import logging +from pathlib import Path +from typing import List + +from ..utils.doc_index import get_or_build_doc_index +from ..utils.ui import ui + +logger = logging.getLogger(__name__) + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd +def cmd_doc_index(argv: List[str]) -> int: + """Build (or reuse the cached) structural index for a Markdown file.""" + p = argparse.ArgumentParser( + prog="cfs doc-index", + description=( + "Build or reuse a cached heading/section index for a Markdown file, " + "so navigation reads the file's structure once, not once per query." + ), + ) + p.add_argument("file", help="Markdown file path") + p.add_argument( + "--rebuild", + action="store_true", + help="Force a fresh build even if a valid cached index exists", + ) + args = p.parse_args(argv) + + filepath = Path(args.file).resolve() + if not filepath.is_file(): + ui.result( + {"file": str(filepath), "status": "ERROR", "message": "File not found"}, + human_fn=lambda d: ui.error(f"{d['file']}: {d['message']}"), + ) + return 2 + + try: + index = get_or_build_doc_index(filepath, force_rebuild=args.rebuild) + except (OSError, UnicodeDecodeError) as exc: + logger.warning("doc-index: cannot read %s: %s", filepath, exc) + ui.result( + {"file": str(filepath), "status": "ERROR", "message": f"Cannot read file: {exc}"}, + human_fn=lambda d: ui.error(f"{d['file']}: {d['message']}"), + ) + return 2 + + output = { + "file": str(filepath), + "cache_hit": index["cache_hit"], + "total_lines": index["total_lines"], + "section_count": len(index["sections"]), + "sections": index["sections"], + } + ui.result(output, human_fn=_human_doc_index) + return 0 +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd-format +def _human_doc_index(data: dict) -> None: + ui.header("Doc Index") + ui.substep(data["file"]) + hit = "cache hit — reused existing index" if data["cache_hit"] else "cache miss — built fresh index" + ui.substep(hit) + ui.substep(f"{data['section_count']} section(s), {data['total_lines']} total lines") + for s in data["sections"]: + summary = f" — {s['summary']}" if s.get("summary") else "" + ui.substep(f" H{s['level']} [{s['line_start']}-{s['line_end']}] {s['heading']}{summary}") + ui.blank() +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cmd-format diff --git a/skills/studio/scripts/studio/commands/validate_toc.py b/skills/studio/scripts/studio/commands/validate_toc.py index 0a7d719e..e0404141 100644 --- a/skills/studio/scripts/studio/commands/validate_toc.py +++ b/skills/studio/scripts/studio/commands/validate_toc.py @@ -14,7 +14,7 @@ from pathlib import Path from typing import List -from ..utils.toc import add_toc_max_level_argument, validate_toc +from ..utils.toc import DEFAULT_MAX_SECTION_LINES, add_toc_max_level_argument, validate_toc from ..utils.ui import ui # @cpt-end:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-imports @@ -31,6 +31,12 @@ def cmd_validate_toc(argv: List[str]) -> int: help="Markdown file path(s) to validate", ) add_toc_max_level_argument(p) + p.add_argument( + "--max-section-lines", + type=int, + default=DEFAULT_MAX_SECTION_LINES, + help=f"Warn when a section exceeds this many lines (default: {DEFAULT_MAX_SECTION_LINES})", + ) p.add_argument( "--verbose", action="store_true", @@ -63,6 +69,7 @@ def cmd_validate_toc(argv: List[str]) -> int: content, artifact_path=filepath, max_heading_level=args.max_level, + max_section_lines=args.max_section_lines, ) errors = report.get("errors", []) @@ -123,6 +130,10 @@ def _human_validate_toc(data: dict) -> None: ui.substep(f" ✗ {e}") for w in r.get("warnings", []): ui.substep(f" ⚠ {w}") + elif status == "WARN": + ui.warn(f"{path}: {warns} warning(s)") + for w in r.get("warnings", []): + ui.substep(f" ⚠ {w}") else: ui.substep(f"{path}: {status}") overall = data.get("status", "") diff --git a/skills/studio/scripts/studio/utils/doc_index.py b/skills/studio/scripts/studio/utils/doc_index.py new file mode 100644 index 00000000..4552c533 --- /dev/null +++ b/skills/studio/scripts/studio/utils/doc_index.py @@ -0,0 +1,257 @@ +"""Cached, read-once-per-file document index for Markdown JIT retrieval. + +Builds a structural index (headings + section line ranges) for a Markdown +file exactly once, persists it keyed by an etag of the file's own state, and +reuses that cached index on every subsequent call against the same file -- +until the file actually changes. This is the "read once per file, not once +per query" mechanism: parsing/etag work never repeats across queries, and +optional per-section summaries (written by an LLM caller, not by this +module) accumulate in the same cached artifact instead of being +re-derived each time. + +Scope: Markdown only. PDF/DOCX conversion is a separate concern (Layer 1); +this module operates purely on already-plain-text content (Layer 2). + +See constructorfabric/studio#104. + +@cpt-algo:cpt-studio-algo-traceability-validation-doc-index:p1 +""" + +from __future__ import annotations + +import hashlib +import json +import logging +import os +import time +from pathlib import Path +from typing import Any, Dict, List, Optional + +from .toc import parse_headings_with_lines + +logger = logging.getLogger(__name__) + +_CACHE_SUBDIR = ".cache" +_INDEX_CACHE_DIR = "doc-index" + +#: Bumped whenever the index's own shape changes incompatibly. Checked +#: alongside the etag so a future schema change invalidates an +#: old-format cache instead of silently returning old-shape data past a +#: matching etag. +_SCHEMA_VERSION = 1 + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-etag +def _compute_etag(path: Path) -> str: + """Compute a cheap cache-validity fingerprint from filesystem metadata. + + Deliberately *not* a content hash: ``Path.stat()`` is metadata-only (no + file read), which is what lets a cache *hit* stay free of a full read -- + the whole point of a read-once-per-file index. mtime + size changes on + a same-size, same-line-count text swap too, since a write ordinarily + advances mtime -- a byte-count/line-count-only fingerprint would miss + that edit outright, and computing either requires reading the entire + file this check exists to avoid reading. + + Known, accepted limitation: on a filesystem with coarse mtime + resolution (e.g. some FAT32/older-HFS+/NFS configurations), two + same-size edits landing within one mtime tick can share an identical + etag, and a cache hit would then return the first edit's stale data. + Trading that narrow, filesystem-dependent risk for never reading the + file on a cache hit is this module's whole reason to exist; closing it + fully would mean a content hash, which defeats the point. + """ + st = path.stat() + return f"{st.st_mtime_ns}:{st.st_size}" +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-etag + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cache-path +def _index_cache_path(path: Path) -> Optional[Path]: + """Resolve ``/.cache/doc-index/.json`` for a file. + + Resolved from ``path`` itself (not the process's current working + directory), so indexing a file outside the caller's cwd still resolves + -- and always resolves -- the Studio directory that actually owns it. + + Returns ``None`` when no Studio directory can be found (e.g. outside a + Studio-adapted project) -- callers should fall back to an uncached build. + """ + from .files import find_studio_directory + + try: + studio_dir = find_studio_directory(path.resolve().parent) + except OSError as exc: + # A file whose parent can't be stat'd (permissions, a race) is not a + # reason to fail the caller -- just an uncached build, like "no + # Studio directory found". + logger.debug("doc-index cache path lookup skipped for %s: %s", path, exc) + studio_dir = None + if studio_dir is None: + return None + + slug = hashlib.sha256(str(path.resolve()).encode("utf-8")).hexdigest()[:16] + return studio_dir / _CACHE_SUBDIR / _INDEX_CACHE_DIR / f"{slug}.json" +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-cache-path + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-build +def build_doc_index(path: Path) -> Dict[str, Any]: + """Build a fresh structural index for a Markdown file. + + Purely deterministic -- headings, section line ranges, and an etag. + Contains no LLM-generated content; per-section ``summary`` fields start + as ``None`` and are filled in later via :func:`annotate_section_summary`. + """ + canonical_path = path.resolve() + # Fingerprint before read: if a write lands between the two, the stored + # etag describes content older than (never newer than) what got parsed, + # so a mismatch is always detected on the next load -- computing it + # after the read could instead capture a fingerprint newer than the + # content actually parsed, which a later stat comparison can't catch. + etag = _compute_etag(canonical_path) + content = canonical_path.read_text(encoding="utf-8") + lines = content.split("\n") + line_count = len(lines) + + headings = parse_headings_with_lines(lines) + sections: List[Dict[str, Any]] = [] + for i, (level, text, line_start) in enumerate(headings): + line_end = headings[i + 1][2] - 1 if i + 1 < len(headings) else line_count + sections.append({ + "level": level, + "heading": text, + "line_start": line_start, + "line_end": line_end, + "summary": None, + }) + + return { + "schema_version": _SCHEMA_VERSION, + "path": str(canonical_path), + "etag": etag, + "built_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()), + "total_lines": line_count, + "sections": sections, + } +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-build + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-load +_REQUIRED_INDEX_FIELDS = ("total_lines", "sections") + + +def _has_schema_current_index(cached: Dict[str, Any]) -> bool: + """``True`` only if ``cached`` carries every field a consumer + (``commands/doc_index.py``, :func:`annotate_section_summary`) reads by + subscript, at the schema version this module currently writes -- + treated the same as a stale/corrupt cache otherwise, so a partially + written, hand-edited, or pre-schema-bump cache triggers a clean rebuild + instead of a ``KeyError`` deep in a consumer. + """ + if cached.get("schema_version") != _SCHEMA_VERSION: + return False + return all(field in cached for field in _REQUIRED_INDEX_FIELDS) + + +def load_doc_index(path: Path) -> Optional[Dict[str, Any]]: + """Load a cached index for ``path``, or ``None`` if missing/stale/absent. + + Staleness is detected from cheap ``Path.stat()`` metadata alone -- this + never reads the file's content, so a cache *hit* stays free of a full + read (the property the whole cache exists to provide). Only a stale or + absent cache falls through to :func:`build_doc_index`, which does the + one real read. + """ + cache_path = _index_cache_path(path) + if cache_path is None or not cache_path.is_file(): + return None + + try: + cached = json.loads(cache_path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError) as exc: + logger.debug("doc-index cache unreadable for %s: %s", path, exc) + return None + + canonical_path = path.resolve() + try: + current_etag = _compute_etag(canonical_path) + except OSError as exc: + logger.debug("doc-index staleness check failed for %s: %s", path, exc) + return None + + if cached.get("etag") != current_etag: + return None + if not _has_schema_current_index(cached): + logger.debug("doc-index cache for %s is malformed or predates the current schema; rebuilding", path) + return None + return cached +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-load + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-save +def save_doc_index(path: Path, index: Dict[str, Any]) -> None: + """Persist an index to its cache location. No-ops outside a Studio project. + + Written atomically (temp file + ``os.replace``): a reader racing a + concurrent writer sees either the old complete file or the new complete + one, never a torn/partial write. + """ + cache_path = _index_cache_path(path) + if cache_path is None: + return + cache_path.parent.mkdir(parents=True, exist_ok=True) + tmp_path = cache_path.with_name(f"{cache_path.name}.{os.getpid()}.tmp") + tmp_path.write_text(json.dumps(index, indent=2), encoding="utf-8") + os.replace(tmp_path, cache_path) +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-save + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-get-or-build +def get_or_build_doc_index(path: Path, *, force_rebuild: bool = False) -> Dict[str, Any]: + """Return the cached index for ``path``, building and caching it if needed. + + This is the "read once per file" entrypoint: the first call for a given + file (or the first call after it changes) pays the parse cost and writes + the cache; every subsequent call against an unchanged file returns the + cached result directly. ``index["cache_hit"]`` reports which happened, + for benchmarking. + """ + if not force_rebuild: + cached = load_doc_index(path) + if cached is not None: + cached["cache_hit"] = True + return cached + + fresh = build_doc_index(path) + save_doc_index(path, fresh) + fresh["cache_hit"] = False + return fresh +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-get-or-build + + +# @cpt-begin:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-annotate +def annotate_section_summary(path: Path, line_start: int, summary: str) -> bool: + """Attach a one-line summary to a cached section, keyed by its line_start. + + Summaries are written by an LLM caller during a one-time enrichment + pass, never generated inside this module. Returns ``False`` when no + valid (non-stale) cached index exists or no section matches + ``line_start`` -- callers should build the index first. + """ + index = load_doc_index(path) + if index is None: + return False + + matched = False + for section in index["sections"]: + if section["line_start"] == line_start: + section["summary"] = summary + matched = True + break + if not matched: + return False + + save_doc_index(path, index) + return True +# @cpt-end:cpt-studio-algo-traceability-validation-doc-index:p1:inst-doc-index-annotate diff --git a/skills/studio/scripts/studio/utils/error_codes.py b/skills/studio/scripts/studio/utils/error_codes.py index 3a45a4ad..8800c40a 100644 --- a/skills/studio/scripts/studio/utils/error_codes.py +++ b/skills/studio/scripts/studio/utils/error_codes.py @@ -111,6 +111,12 @@ TOC_HEADING_NOT_IN_TOC = "toc-heading-not-in-toc" TOC_STALE = "toc-stale" +# JIT-retrieval readiness signals (warning-only) — see constructorfabric/studio#104 +TOC_HEADING_DUPLICATE = "toc-heading-duplicate" +TOC_HEADING_DEPTH_JUMP = "toc-heading-depth-jump" +TOC_SECTION_TOO_LONG = "toc-section-too-long" +TOC_MISSING_DESCRIPTION = "toc-missing-description" + # --------------------------------------------------------------------------- # File errors # --------------------------------------------------------------------------- diff --git a/skills/studio/scripts/studio/utils/toc.py b/skills/studio/scripts/studio/utils/toc.py index 3662ac2c..daf6a711 100644 --- a/skills/studio/scripts/studio/utils/toc.py +++ b/skills/studio/scripts/studio/utils/toc.py @@ -22,6 +22,8 @@ import re import argparse +import math +import unicodedata from pathlib import Path from typing import Any, Dict, List, Optional, Tuple @@ -134,13 +136,54 @@ def parse_headings( max_level: Maximum heading level to include. skip_first: If True, skip the very first heading (document title). skip_toc_heading: If True, skip headings named "Table of Contents" or "TOC". + + Thin wrapper over :func:`parse_headings_with_lines` (stripping the line + number): one fence-tracking/heading-matching implementation instead of + two that could silently diverge. + """ + return [ + (level, text) + for level, text, _line in parse_headings_with_lines( + lines, + min_level=min_level, + max_level=max_level, + skip_first=skip_first, + skip_toc_heading=skip_toc_heading, + ) + ] +# @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-parse-headings + +# @cpt-begin:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-parse-headings-lines +def parse_headings_with_lines( + lines: List[str], + *, + min_level: int = 1, + max_level: int = 6, + skip_first: bool = False, + skip_toc_heading: bool = False, +) -> List[Tuple[int, str, int]]: + """Extract ``(level, text, line_number)`` triples from markdown lines. + + Fence-aware like :func:`parse_headings` (which delegates here), and + skips a leading YAML front-matter block (see + :func:`_find_frontmatter_end`) so a ``#``-prefixed line inside + front-matter data (a comment, a value) is never mistaken for a real + heading. ``line_number`` is 1-based. + + ``skip_first``/``skip_toc_heading`` mirror :func:`parse_headings`'s own + options: ``skip_first`` drops the very first heading matched + (regardless of level, checked before the level filter, same order as + the original standalone implementation), ``skip_toc_heading`` drops + headings named "Table of Contents"/"TOC" after the level filter. """ - headings: List[Tuple[int, str]] = [] + headings: List[Tuple[int, str, int]] = [] fence: Optional[Tuple[str, int]] = None + frontmatter_end = _find_frontmatter_end(lines) first_skipped = False - for line in lines: - # Track fenced code blocks (``` or ~~~ with 3+ chars) + for idx, line in enumerate(lines): + if idx < frontmatter_end: + continue new_fence = _fence_update(line, fence) if new_fence != fence: fence = new_fence @@ -165,10 +208,10 @@ def parse_headings( if skip_toc_heading and text.lower() in _TOC_HEADING_NAMES: continue - headings.append((level, text)) + headings.append((level, text, idx + 1)) return headings -# @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-parse-headings +# @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-parse-headings-lines # --------------------------------------------------------------------------- # TOC building @@ -687,6 +730,207 @@ def _record_missing_toc_error( )) +# @cpt-begin:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-jit-readiness +# --------------------------------------------------------------------------- +# JIT-retrieval readiness — structural signals beyond TOC correctness +# --------------------------------------------------------------------------- +# These four checks are additive warnings only (never errors): they flag +# structural properties that make a document harder to navigate via +# heading-based just-in-time retrieval, without invalidating documents that +# are otherwise fine. See constructorfabric/studio#104. + +DEFAULT_MAX_SECTION_LINES = 300 +# Below this size, a missing description is not worth flagging — the whole +# point of a description is to let a caller pick the right *file* before +# reading it, among many; a trivial file doesn't need that. +MIN_LINES_FOR_DESCRIPTION_CHECK = 100 + + +def _normalize_heading_key(text: str) -> str: + """Fold a heading's text to a comparison key for duplicate detection. + + Casefolds, collapses internal whitespace runs to a single space, and + NFC-normalizes so two headings that render identically -- differing + only in case, incidental whitespace, or Unicode composition -- are + still recognized as the same title. The original text is kept for + display; only the comparison key is normalized. + """ + return unicodedata.normalize("NFC", " ".join(text.split())).casefold() + + +def _check_duplicate_heading_titles( + headings_with_lines: List[Tuple[int, str, int]], + path: Path, +) -> List[Dict[str, Any]]: + """Warn when the same heading text appears more than once. + + Duplicate titles are tolerated by anchor-suffixing elsewhere (see + ``_unique_slug``) and are NOT errors, but they make it impossible to + unambiguously address a section by its heading text alone. + """ + from . import error_codes as EC + from .constraints import error + + seen: Dict[str, int] = {} + warnings: List[Dict[str, Any]] = [] + for _level, text, line in headings_with_lines: + key = _normalize_heading_key(text) + if key in seen: + warnings.append(error( + "toc", + f"Heading `{text}` duplicates an earlier heading (first seen at line {seen[key]})", + code=EC.TOC_HEADING_DUPLICATE, + path=path, + line=line, + heading_text=text, + first_seen_line=seen[key], + )) + else: + seen[key] = line + return warnings + + +def _check_heading_depth_jumps( + headings_with_lines: List[Tuple[int, str, int]], + path: Path, +) -> List[Dict[str, Any]]: + """Warn when heading depth increases by more than one level at once. + + E.g. an H2 followed directly by an H4 skips H3 — this breaks the + "read from this heading to the next heading at the same or higher + level" boundary computation JIT retrieval relies on. + """ + from . import error_codes as EC + from .constraints import error + + warnings: List[Dict[str, Any]] = [] + prev_level: Optional[int] = None + for level, text, line in headings_with_lines: + if prev_level is not None and level > prev_level + 1: + warnings.append(error( + "toc", + f"Heading `{text}` jumps from H{prev_level} to H{level}, skipping intermediate level(s)", + code=EC.TOC_HEADING_DEPTH_JUMP, + path=path, + line=line, + heading_text=text, + from_level=prev_level, + to_level=level, + )) + prev_level = level + return warnings + + +def _check_section_lengths( + headings_with_lines: List[Tuple[int, str, int]], + total_lines: int, + path: Path, + max_section_lines: int, +) -> List[Dict[str, Any]]: + """Warn when a section's body (up to the next heading, any level) is too long. + + An oversized section with no sub-headings defeats heading-based JIT + retrieval: reading "one section" still means reading the whole thing. + + ``max_section_lines`` is validated here, independent of any CLI + argparse guard: a non-finite value (``nan``/``inf``) or a non-positive + one falls back to :data:`DEFAULT_MAX_SECTION_LINES` rather than + silently disabling the check (``nan``) or flagging virtually every + section (a negative threshold) for a direct library caller. + """ + from . import error_codes as EC + from .constraints import error + + if not math.isfinite(max_section_lines) or max_section_lines <= 0: + max_section_lines = DEFAULT_MAX_SECTION_LINES + + warnings: List[Dict[str, Any]] = [] + for i, (_level, text, line) in enumerate(headings_with_lines): + next_line = ( + headings_with_lines[i + 1][2] + if i + 1 < len(headings_with_lines) + else total_lines + 1 + ) + section_length = next_line - line + if section_length > max_section_lines: + warnings.append(error( + "toc", + f"Section `{text}` is {section_length} lines long (max recommended: {max_section_lines})", + code=EC.TOC_SECTION_TOO_LONG, + path=path, + line=line, + heading_text=text, + section_length=section_length, + max_section_lines=max_section_lines, + )) + return warnings + + +_DESCRIPTION_FIELD_RE = re.compile(r"^description\s*:\s*(.*)$") + + +def _is_real_description_value(raw_value: str) -> bool: + """``True`` only if *raw_value* (the text after ``description:``) is an + actual description, not an empty quoted scalar (``""``/``''``) or a + comment-only value (``# TODO``) -- both look non-blank to a naive + "any character after the colon" check but carry no real content. + """ + value = raw_value.strip() + if not value or value.startswith("#"): + return False + if value[0] in "\"'": + quote = value[0] + end = value.find(quote, 1) + quoted = value[1:end] if end != -1 else value[1:] + return bool(quoted.strip()) + return True + + +def _frontmatter_has_description(lines: List[str], frontmatter_end: int) -> bool: + """Check whether a YAML frontmatter block declares a non-empty ``description``. + + ``frontmatter_end`` is the index returned by :func:`_find_frontmatter_end` + (one past the closing ``---``); the body being scanned is + ``lines[1:frontmatter_end - 1]``, excluding both delimiter lines. + """ + for line in lines[1:frontmatter_end - 1]: + match = _DESCRIPTION_FIELD_RE.match(line.strip()) + if match and _is_real_description_value(match.group(1)): + return True + return False + + +def _check_missing_description( + lines: List[str], + path: Path, +) -> List[Dict[str, Any]]: + """Warn when a document has no frontmatter block with a real description. + + A short description lets a caller pick the right *document* before + reading any of its headings — the same principle as heading + descriptiveness, one level up. Frontmatter that exists but carries no + ``description`` field (e.g. only a ``title``) does not satisfy this — + an empty promise is the same as no promise. Only checked above + ``MIN_LINES_FOR_DESCRIPTION_CHECK`` lines — a trivial file doesn't need + a description, and flagging every small file drowns the signal. + """ + from . import error_codes as EC + from .constraints import error + + if len(lines) < MIN_LINES_FOR_DESCRIPTION_CHECK: + return [] + frontmatter_end = _find_frontmatter_end(lines) + if frontmatter_end > 0 and _frontmatter_has_description(lines, frontmatter_end): + return [] + return [error( + "toc", + "Document has no frontmatter/description block at the top", + code=EC.TOC_MISSING_DESCRIPTION, + path=path, + line=1, + )] +# @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-jit-readiness + # @cpt-begin:cpt-studio-algo-traceability-validation-validate-toc:p1:inst-toc-compare def _validate_toc_entries( toc_entries: List[Tuple[str, str, int]], @@ -764,12 +1008,38 @@ def _append_stale_toc_warning( )) # @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-helpers +# @cpt-begin:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-jit-readiness-collect +def _collect_jit_readiness_warnings( + lines: List[str], + path: Path, + max_section_lines: int, +) -> List[Dict[str, Any]]: + """Gather all four JIT-retrieval readiness warnings for a document. + + Always parses *every* heading level, independent of whatever + ``max_heading_level`` the caller configured for TOC-completeness + checking above — these signals are about the document's real structure + (would a duplicate/depth-jump/oversized-section problem trip up + heading-based retrieval), not about which levels belong in a + human-authored TOC. Filtering by the TOC's level cap would hide real H4-H6 + issues under a shallow default (e.g. the CLI's own ``--max-level 3``). + """ + warnings: List[Dict[str, Any]] = [] + warnings.extend(_check_missing_description(lines, path)) + all_headings = parse_headings_with_lines(lines) + warnings.extend(_check_duplicate_heading_titles(all_headings, path)) + warnings.extend(_check_heading_depth_jumps(all_headings, path)) + warnings.extend(_check_section_lengths(all_headings, len(lines), path, max_section_lines)) + return warnings +# @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-jit-readiness-collect + # @cpt-begin:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-validate def validate_toc( content: str, *, artifact_path: Optional[Path] = None, max_heading_level: int = 6, + max_section_lines: int = DEFAULT_MAX_SECTION_LINES, ) -> Dict[str, List[Dict[str, Any]]]: """Validate the Table of Contents in a markdown document. @@ -784,6 +1054,11 @@ def validate_toc( 4. **Freshness** — if the TOC were regenerated, it would match the current content (catches reordering / renamed headings). + Plus four additive, warning-only JIT-retrieval readiness signals that + run regardless of TOC presence/errors above (see constructorfabric/studio#104): + duplicate heading titles, heading depth jumps, oversized sections, and a + missing top-of-file description/frontmatter block. + Returns ``{"errors": [...], "warnings": [...]}`` in the same format as ``validate_artifact_file``. """ @@ -796,8 +1071,12 @@ def validate_toc( max_heading_level, ) + # JIT-retrieval readiness signals (warning-only, run regardless of + # TOC presence below — independent of the TOC-filtered `headings`). + warnings.extend(_collect_jit_readiness_warnings(lines, path, max_section_lines)) + if not headings: - # No headings → nothing to validate + # No headings → nothing further to validate return {"errors": errors, "warnings": warnings} # @cpt-end:cpt-studio-algo-traceability-validation-toc-utils:p1:inst-toc-util-validate-init diff --git a/tests/test_doc_index.py b/tests/test_doc_index.py new file mode 100644 index 00000000..888b101d --- /dev/null +++ b/tests/test_doc_index.py @@ -0,0 +1,449 @@ +"""Tests for the cached, read-once-per-file document index (doc_index.py). + +See constructorfabric/studio#104. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path + +import pytest + +from studio.commands.doc_index import cmd_doc_index +from studio.utils.doc_index import ( + annotate_section_summary, + build_doc_index, + get_or_build_doc_index, + load_doc_index, + save_doc_index, +) + +_SAMPLE = ( + "# Title\n\n" + "## Section A\n\n" + "Body of A.\n\n" + "### A.1\n\n" + "Body of A.1.\n\n" + "## Section B\n\n" + "Body of B.\n" +) + + +def _write(tmp_path: Path, content: str = _SAMPLE, name: str = "doc.md") -> Path: + f = tmp_path / name + f.write_text(content, encoding="utf-8") + return f + + +class TestBuildDocIndex: + def test_extracts_sections_with_line_ranges(self, tmp_path: Path): + f = _write(tmp_path) + index = build_doc_index(f) + headings = [(s["level"], s["heading"], s["line_start"], s["line_end"]) for s in index["sections"]] + assert headings == [ + (1, "Title", 1, 2), + (2, "Section A", 3, 6), + (3, "A.1", 7, 10), + (2, "Section B", 11, 14), + ] + + def test_sections_start_with_no_summary(self, tmp_path: Path): + f = _write(tmp_path) + index = build_doc_index(f) + assert all(s["summary"] is None for s in index["sections"]) + + def test_etag_present_and_stable_for_same_content(self, tmp_path: Path): + f = _write(tmp_path) + idx1 = build_doc_index(f) + idx2 = build_doc_index(f) + assert idx1["etag"] == idx2["etag"] + + def test_etag_changes_when_content_changes(self, tmp_path: Path): + f = _write(tmp_path) + idx1 = build_doc_index(f) + f.write_text(_SAMPLE + "\n## Section C\n") + idx2 = build_doc_index(f) + assert idx1["etag"] != idx2["etag"] + + def test_etag_is_computed_before_reading_content(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #108 (round 2): the etag must be captured before the + content read, not after -- a write landing between the two calls + would otherwise let the stored etag describe content newer than + what got parsed, and no later stat comparison could ever detect + that mismatch. Computing the etag first means a race can only make + it look older than the parsed content, which a later check always + catches.""" + from studio.utils import doc_index as doc_index_module + + f = _write(tmp_path) + call_order = [] + + real_compute_etag = doc_index_module._compute_etag + real_read_text = Path.read_text + + def _tracked_compute_etag(path): + call_order.append("etag") + return real_compute_etag(path) + + def _tracked_read_text(self, *args, **kwargs): + call_order.append("read") + return real_read_text(self, *args, **kwargs) + + monkeypatch.setattr(doc_index_module, "_compute_etag", _tracked_compute_etag) + monkeypatch.setattr(Path, "read_text", _tracked_read_text) + + doc_index_module.build_doc_index(f) + assert call_order == ["etag", "read"] + + def test_skips_headings_in_fenced_code(self, tmp_path: Path): + content = "# Title\n\n## Real\n\n```bash\n# not a heading\n```\n\n## Also Real\n" + f = _write(tmp_path, content) + index = build_doc_index(f) + assert [s["heading"] for s in index["sections"]] == ["Title", "Real", "Also Real"] + + +class TestCachePersistence: + def test_load_returns_none_when_no_cache_exists(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert load_doc_index(f) is None + + def test_save_then_load_round_trips(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + built = build_doc_index(f) + save_doc_index(f, built) + loaded = load_doc_index(f) + assert loaded is not None + assert loaded["etag"] == built["etag"] + assert loaded["sections"] == built["sections"] + + def test_load_returns_none_when_cache_is_stale(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + f.write_text(_SAMPLE + "\n## Section C\n") # content changed after caching + assert load_doc_index(f) is None + + def test_same_size_same_line_count_edit_is_still_detected_as_stale( + self, tmp_path: Path, monkeypatch + ): + """Regression test (see PR #108 review): a same-size, same-line-count + content swap must still invalidate the cache. A byte-size + + line-count fingerprint alone cannot distinguish this from an + unchanged file -- mtime can, since a real write always advances it. + """ + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + + edited = _SAMPLE.replace("Section A", "Section Z") + assert len(edited) == len(_SAMPLE) + assert edited.count("\n") == _SAMPLE.count("\n") + f.write_text(edited, encoding="utf-8") + # Force a distinct mtime regardless of filesystem clock resolution -- + # the mechanism under test is "mtime changed", not "enough wall-clock + # time elapsed during the test run". + st = f.stat() + os.utime(f, ns=(st.st_atime_ns, st.st_mtime_ns + 1)) + + assert load_doc_index(f) is None + fresh = get_or_build_doc_index(f) + assert any(s["heading"] == "Section Z" for s in fresh["sections"]) + + def test_studio_directory_resolved_from_file_path_not_cwd( + self, tmp_path: Path, monkeypatch + ): + """CodeRabbit PR #108: the Studio directory must be resolved from the + indexed file's own location, not the process's cwd -- otherwise + indexing a file outside the caller's cwd can miss or mis-target the + cache.""" + seen_paths = [] + + def _spy(start_path): + seen_paths.append(start_path) + return tmp_path + + monkeypatch.setattr("studio.utils.files.find_studio_directory", _spy) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + + assert seen_paths, "find_studio_directory was never called" + assert seen_paths[0] == f.resolve().parent + + def test_load_returns_none_on_corrupt_cache_file(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + # Corrupt the cache file directly + cache_dir = tmp_path / ".cache" / "doc-index" + for cache_file in cache_dir.glob("*.json"): + cache_file.write_text("{not valid json", encoding="utf-8") + assert load_doc_index(f) is None + + def test_cmd_doc_index_rebuilds_cleanly_after_corrupt_cache(self, tmp_path: Path, capsys, monkeypatch): + """CodeRabbit PR #108: prove the corrupt-cache fallback at the + CLI/exit-code level, not just load_doc_index() in isolation.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + cache_dir = tmp_path / ".cache" / "doc-index" + for cache_file in cache_dir.glob("*.json"): + cache_file.write_text("{not valid json", encoding="utf-8") + + rc = cmd_doc_index([str(f)]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["cache_hit"] is False + + def test_cache_missing_a_required_field_is_rebuilt_not_returned(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #108: a matching-etag cache missing "sections" + (hand-edited, or truncated mid-write) used to pass load_doc_index's + etag-only check and reach cmd_doc_index()'s len(index["sections"]) + as an unhandled KeyError.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + incomplete = build_doc_index(f) + del incomplete["sections"] + save_doc_index(f, incomplete) + + assert load_doc_index(f) is None + index = get_or_build_doc_index(f) + assert index["cache_hit"] is False + assert "sections" in index + + def test_cache_from_an_older_schema_version_is_rebuilt_not_returned(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + old_schema = build_doc_index(f) + old_schema["schema_version"] = 0 + save_doc_index(f, old_schema) + + assert load_doc_index(f) is None + index = get_or_build_doc_index(f) + assert index["cache_hit"] is False + + def test_save_does_not_leave_a_temp_file_behind(self, tmp_path: Path, monkeypatch): + """CodeRabbit PR #108: save_doc_index() writes atomically (temp + file + os.replace) -- the temp file must not survive a successful + write.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + cache_dir = tmp_path / ".cache" / "doc-index" + names = [p.name for p in cache_dir.iterdir()] + assert all(name.endswith(".json") for name in names) + assert load_doc_index(f) is not None + + def test_no_studio_directory_means_no_crash_and_always_none(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: None) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) # must no-op silently, not raise + assert load_doc_index(f) is None + + def test_studio_directory_lookup_error_means_no_crash_and_no_cache( + self, tmp_path: Path, monkeypatch + ): + """An OSError from find_studio_directory (e.g. an unreadable parent + directory) must degrade to 'no cache', not raise -- and it must be + logged, not silently swallowed (see PR #108 review / pylint W9001).""" + def _raise(_start_path): + raise OSError("permission denied") + + monkeypatch.setattr("studio.utils.files.find_studio_directory", _raise) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) # must no-op, not raise + assert load_doc_index(f) is None + + def test_load_returns_none_when_file_deleted_after_caching(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + save_doc_index(f, build_doc_index(f)) + f.unlink() + assert load_doc_index(f) is None + + +class TestGetOrBuildDocIndex: + def test_first_call_is_cache_miss(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + index = get_or_build_doc_index(f) + assert index["cache_hit"] is False + + def test_second_call_is_cache_hit(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + get_or_build_doc_index(f) + index = get_or_build_doc_index(f) + assert index["cache_hit"] is True + + def test_cache_hit_preserves_previously_annotated_summary(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + get_or_build_doc_index(f) + assert annotate_section_summary(f, line_start=3, summary="Covers A.") is True + index = get_or_build_doc_index(f) + assert index["cache_hit"] is True + section_a = next(s for s in index["sections"] if s["heading"] == "Section A") + assert section_a["summary"] == "Covers A." + + def test_content_change_invalidates_and_drops_stale_summaries(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + get_or_build_doc_index(f) + annotate_section_summary(f, line_start=3, summary="Covers A.") + f.write_text(_SAMPLE + "\n## Section C\n") + index = get_or_build_doc_index(f) + assert index["cache_hit"] is False + assert all(s["summary"] is None for s in index["sections"]) + + def test_force_rebuild_bypasses_valid_cache(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + get_or_build_doc_index(f) + index = get_or_build_doc_index(f, force_rebuild=True) + assert index["cache_hit"] is False + + +class TestAnnotateSectionSummary: + def test_returns_false_when_no_cache_exists_yet(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + assert annotate_section_summary(f, line_start=3, summary="x") is False + + def test_returns_false_for_unmatched_line_start(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + get_or_build_doc_index(f) + assert annotate_section_summary(f, line_start=999, summary="x") is False + + def test_returns_true_and_persists_on_match(self, tmp_path: Path, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + get_or_build_doc_index(f) + assert annotate_section_summary(f, line_start=1, summary="The title.") is True + cached = load_doc_index(f) + assert cached["sections"][0]["summary"] == "The title." + + +class TestCmdDocIndex: + def test_missing_file(self, tmp_path: Path, capsys): + rc = cmd_doc_index([str(tmp_path / "nope.md")]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_non_utf8_file_reports_a_clean_error_not_a_raw_traceback( + self, tmp_path: Path, capsys, monkeypatch + ): + """CodeRabbit PR #108 (round 2): a non-UTF-8 file raises + UnicodeDecodeError inside build_doc_index's read_text -- this must + surface as a clean exit-code-2 JSON error, the same contract a + missing file already gets, not an unhandled traceback.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = tmp_path / "bad.md" + f.write_bytes(b"# Title\n\xff\xfe not valid utf-8\n") + rc = cmd_doc_index([str(f)]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_unreadable_file_reports_a_clean_error_not_a_raw_traceback( + self, tmp_path: Path, capsys, monkeypatch + ): + """CodeRabbit PR #108 (round 2): an OSError from get_or_build_doc_index + (e.g. a permissions failure or a race where the file vanishes after + the is_file() check) must use the same clean error contract as a + UnicodeDecodeError, not escape as a raw traceback.""" + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + + def _raise_os_error(*_a, **_k): + raise OSError("permission denied") + + monkeypatch.setattr("studio.commands.doc_index.get_or_build_doc_index", _raise_os_error) + rc = cmd_doc_index([str(f)]) + assert rc == 2 + out = json.loads(capsys.readouterr().out) + assert out["status"] == "ERROR" + + def test_basic(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + rc = cmd_doc_index([str(f)]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["cache_hit"] is False + assert out["section_count"] == 4 + + def test_second_invocation_is_cache_hit(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + cmd_doc_index([str(f)]) + capsys.readouterr() + rc = cmd_doc_index([str(f)]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["cache_hit"] is True + + def test_rebuild_flag_forces_cache_miss(self, tmp_path: Path, capsys, monkeypatch): + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + cmd_doc_index([str(f)]) + capsys.readouterr() + rc = cmd_doc_index([str(f), "--rebuild"]) + assert rc == 0 + out = json.loads(capsys.readouterr().out) + assert out["cache_hit"] is False + + def test_human_output_mode(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_doc_index([str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "Doc Index" in out + assert "cache miss" in out + assert "Section A" in out + + def test_human_output_mode_cache_hit(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + cmd_doc_index([str(f)]) + capsys.readouterr() + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_doc_index([str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "cache hit" in capsys.readouterr().out + + def test_human_output_mode_with_section_summary(self, tmp_path: Path, capsys, monkeypatch): + from studio.utils.ui import is_json_mode, set_json_mode + + monkeypatch.setattr("studio.utils.files.find_studio_directory", lambda *_a, **_k: tmp_path) + f = _write(tmp_path) + cmd_doc_index([str(f)]) + capsys.readouterr() + assert annotate_section_summary(f, line_start=3, summary="Covers A.") is True + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_doc_index([str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + assert "Covers A." in capsys.readouterr().out diff --git a/tests/test_toc.py b/tests/test_toc.py index 71d5504a..6d6e8865 100644 --- a/tests/test_toc.py +++ b/tests/test_toc.py @@ -707,6 +707,372 @@ def test_toml_comments_in_fence_between_toc_and_heading(self): assert result["errors"] == [], f"Unexpected errors: {result['errors']}" +# --------------------------------------------------------------------------- +# JIT-retrieval readiness signals (constructorfabric/studio#104) +# --------------------------------------------------------------------------- + +class TestJitRetrievalReadiness: + def test_duplicate_heading_titles_warned(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [Intro](#intro)\n" + "2. [Intro](#intro-1)\n\n" + "---\n\n" + "## Intro\n\n" + "## Intro\n" + ) + result = validate_toc(content, max_heading_level=2) + assert result["errors"] == [] + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-duplicate" in codes + dup = [w for w in result["warnings"] if w["code"] == "toc-heading-duplicate"][0] + assert dup["heading_text"] == "Intro" + assert dup["first_seen_line"] == 10 + + def test_no_duplicate_warning_for_unique_headings(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n" + "2. [B](#b)\n\n" + "---\n\n" + "## A\n\n" + "## B\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-duplicate" not in codes + + def test_duplicate_detection_is_case_insensitive(self): + """CodeRabbit PR #108: "Section" and "section" render identically + to a reader but compared unequal under a raw dict key.""" + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [Section](#section)\n" + "2. [section](#section-1)\n\n" + "---\n\n" + "## Section\n\n" + "## section\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-duplicate" in codes + + def test_duplicate_detection_collapses_internal_whitespace(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [Setup Guide](#setup--guide)\n" + "2. [Setup Guide](#setup-guide)\n\n" + "---\n\n" + "## Setup Guide\n\n" + "## Setup Guide\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-duplicate" in codes + + def test_duplicate_warning_still_shows_the_original_heading_text(self): + """Normalizing the comparison key must not leak into the warning's + display text -- a reader needs to see the heading as written.""" + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [SECTION](#section)\n" + "2. [section](#section-1)\n\n" + "---\n\n" + "## SECTION\n\n" + "## section\n" + ) + result = validate_toc(content, max_heading_level=2) + dup = [w for w in result["warnings"] if w["code"] == "toc-heading-duplicate"][0] + assert dup["heading_text"] == "section" + + def test_frontmatter_hash_line_is_not_parsed_as_a_heading(self): + """CodeRabbit PR #108: a `#`-prefixed line inside YAML front-matter + (a comment, or a value starting with `#`) must not be mistaken for + a real heading. Diagnostic: the front-matter line's text matches + the one real heading below it -- if front-matter weren't skipped, + it would register as a fake first occurrence and the real heading + would incorrectly warn as its "duplicate".""" + content = ( + "---\n" + "title: Foo\n" + "# Section\n" + "---\n" + "# Title\n\n" + "## Table of Contents\n\n" + "1. [Section](#section)\n\n" + "---\n\n" + "## Section\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-duplicate" not in codes + + def test_depth_jump_warned(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + "## A\n\n" + "#### Skipped H3\n" + ) + result = validate_toc(content, max_heading_level=4) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-depth-jump" in codes + jump = [w for w in result["warnings"] if w["code"] == "toc-heading-depth-jump"][0] + assert jump["from_level"] == 2 + assert jump["to_level"] == 4 + + def test_no_depth_jump_warning_for_consecutive_levels(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + "## A\n\n" + "### A.1\n" + ) + result = validate_toc(content, max_heading_level=3) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-depth-jump" not in codes + + def test_shallower_heading_not_a_depth_jump(self): + # Going H3 -> H1 (shallower) must never be flagged; only jumps deeper + # by more than one level are a problem. + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + "## A\n\n" + "### A.1\n\n" + "# Back to top level\n" + ) + result = validate_toc(content, max_heading_level=3) + codes = [w["code"] for w in result["warnings"]] + assert "toc-heading-depth-jump" not in codes + + def test_oversized_section_warned(self): + body = "\n".join(f"line {i}" for i in range(400)) + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n" + "2. [B](#b)\n\n" + "---\n\n" + "## A\n\n" + f"{body}\n\n" + "## B\n" + ) + result = validate_toc(content, max_heading_level=2, max_section_lines=300) + codes = [w["code"] for w in result["warnings"]] + assert "toc-section-too-long" in codes + long_section = [w for w in result["warnings"] if w["code"] == "toc-section-too-long"][0] + assert long_section["heading_text"] == "A" + assert long_section["section_length"] > 300 + + def test_nan_max_section_lines_falls_back_to_the_default_instead_of_disabling_the_check(self): + """CodeRabbit PR #108: float('nan') > anything is always False, so + an unguarded nan silently disabled the oversized-section check + entirely for a direct library caller (bypassing the CLI's + argparse(type=int) guard).""" + body = "\n".join(f"line {i}" for i in range(400)) + content = ( + "# Title\n\n## Table of Contents\n\n1. [A](#a)\n\n---\n\n## A\n\n" + body + "\n" + ) + result = validate_toc(content, max_heading_level=2, max_section_lines=float("nan")) + codes = [w["code"] for w in result["warnings"]] + assert "toc-section-too-long" in codes + + def test_negative_max_section_lines_falls_back_to_the_default_instead_of_flagging_everything(self): + content = ( + "# Title\n\n## Table of Contents\n\n1. [A](#a)\n\n---\n\n## A\n\nShort content.\n" + ) + result = validate_toc(content, max_heading_level=2, max_section_lines=-1) + codes = [w["code"] for w in result["warnings"]] + assert "toc-section-too-long" not in codes + + def test_section_within_limit_not_warned(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + "## A\n\n" + "Short content.\n" + ) + result = validate_toc(content, max_heading_level=2, max_section_lines=300) + codes = [w["code"] for w in result["warnings"]] + assert "toc-section-too-long" not in codes + + def test_last_section_length_measured_to_end_of_file(self): + body = "\n".join(f"line {i}" for i in range(400)) + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{body}\n" + ) + result = validate_toc(content, max_heading_level=2, max_section_lines=300) + codes = [w["code"] for w in result["warnings"]] + assert "toc-section-too-long" in codes + + def test_missing_description_warned_above_size_threshold(self): + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{filler}\n" + ) + assert len(content.split("\n")) >= 100 + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" in codes + + def test_missing_description_not_warned_below_size_threshold(self): + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + "## A\n\nShort.\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" not in codes + + def test_frontmatter_present_suppresses_missing_description(self): + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "---\n" + "description: A test document.\n" + "---\n\n" + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{filler}\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" not in codes + + def test_frontmatter_without_description_field_still_warns(self): + """CodeRabbit PR #108: frontmatter existing is not the same as a + description existing -- a block with only unrelated fields (e.g. + title) must still warn, not be silently accepted as satisfying the + check its own name promises.""" + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "---\n" + "title: A test document.\n" + "---\n\n" + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{filler}\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" in codes + + @pytest.mark.parametrize( + "description_line", + [ + 'description: ""', + "description: ''", + "description: # TODO", + ], + ids=["double-quoted-empty", "single-quoted-empty", "comment-only"], + ) + def test_empty_or_comment_only_description_still_warns(self, description_line): + """CodeRabbit PR #108 (round 2): an empty quoted scalar or a bare + comment satisfies a naive "any character after the colon" check but + is not a real description -- each must still trigger the warning.""" + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "---\n" + f"{description_line}\n" + "---\n\n" + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + f"## A\n\n{filler}\n" + ) + result = validate_toc(content, max_heading_level=2) + codes = [w["code"] for w in result["warnings"]] + assert "toc-missing-description" in codes + + def test_jit_readiness_warnings_are_never_errors(self): + # All four signals are additive warnings; they must never appear + # in `errors`, regardless of how badly a document scores. (This + # fixture also trips an unrelated, pre-existing TOC-completeness + # error since "Skipped H2/H3" isn't listed in the TOC — that error + # is expected and irrelevant to what's being asserted here.) + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [Intro](#intro)\n" + "2. [Intro](#intro-1)\n\n" + "---\n\n" + "## Intro\n\n" + f"{filler}\n\n" + "#### Skipped H2/H3\n\n" + "## Intro\n" + ) + result = validate_toc(content, max_heading_level=4) + jit_codes = { + "toc-heading-duplicate", + "toc-heading-depth-jump", + "toc-section-too-long", + "toc-missing-description", + } + error_codes = {e["code"] for e in result["errors"]} + assert not (error_codes & jit_codes), f"JIT-readiness code leaked into errors: {error_codes}" + # This fixture triggers duplicate + depth-jump + missing-description, + # but not section-too-long (its filler is under the 300-line default). + warning_codes = {w["code"] for w in result["warnings"]} + assert {"toc-heading-duplicate", "toc-heading-depth-jump", "toc-missing-description"}.issubset( + warning_codes + ) + + def test_readiness_signals_see_headings_deeper_than_max_heading_level(self): + """CodeRabbit PR #108: readiness checks must see *every* heading + level, independent of max_heading_level (the CLI's own default is + 3). A duplicate/depth-jump/oversized-section problem below that + level must still be caught -- filtering by the TOC's level cap here + would silently hide real structural problems in H4-H6 content, as + it did against a real PDF-converted document during development.""" + body = "\n".join(f"line {i}" for i in range(400)) + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [A](#a)\n\n" + "---\n\n" + "## A\n\n" + "#### Deep\n\n" + f"{body}\n\n" + "#### Deep\n" + ) + # max_heading_level=2: TOC completeness only cares about H1/H2, but + # the two duplicate/oversized H4 "Deep" headings must still surface. + result = validate_toc(content, max_heading_level=2, max_section_lines=300) + warning_codes = {w["code"] for w in result["warnings"]} + assert "toc-heading-duplicate" in warning_codes + assert "toc-section-too-long" in warning_codes + + # --------------------------------------------------------------------------- # cmd_validate_toc (integration) # --------------------------------------------------------------------------- @@ -786,6 +1152,66 @@ def test_warn_stale_toc(self, tmp_path: Path, capsys): assert out["status"] == "WARN" assert out["warning_count"] >= 1 + def test_warn_only_file_prints_warnings_in_human_output(self, tmp_path: Path, capsys, monkeypatch): + """CodeRabbit PR #108: a WARN-only file's human-mode output used to + print just "path: WARN" with no indication of what's wrong -- the + FAIL branch already iterated warnings, but the WARN branch (the + default `else`) never did, making this PR's own headline feature + (JIT-readiness warnings) invisible outside --json.""" + from studio.utils.ui import is_json_mode, set_json_mode + + f = tmp_path / "stale.md" + f.write_text( + "# T\n\n" + "## Table of Contents\n\n" + "1. [B](#b)\n" + "2. [A](#a)\n\n" + "---\n\n" + "## A\n\n" + "## B\n", + encoding="utf-8", + ) + orig = is_json_mode() + set_json_mode(False) + try: + rc = cmd_validate_toc(["--max-level", "2", str(f)]) + finally: + set_json_mode(orig) + assert rc == 0 + out = capsys.readouterr().out + assert "warning(s)" in out + assert "⚠" in out + + def test_all_four_jit_warnings_zero_errors_exits_clean_via_cli(self, tmp_path: Path, capsys): + """CodeRabbit PR #108: prove the warn-only guarantee end to end + through cmd_validate_toc, not just validate_toc() directly -- a + document tripping JIT-readiness codes with an otherwise complete + TOC must still return rc == 0 and status WARN, zero errors. + --max-level 2 keeps the deeper H4 "Sub" heading (which trips the + depth-jump and section-too-long checks -- those see every heading + regardless of max_heading_level, per the readiness checks' own + design) out of TOC-completeness scope, so it needs no TOC entry.""" + filler = "\n\n".join(f"Paragraph {i} of filler text." for i in range(60)) + content = ( + "# Title\n\n" + "## Table of Contents\n\n" + "1. [Intro](#intro)\n" + "2. [Intro](#intro-1)\n\n" + "---\n\n" + "## Intro\n\n" + f"{filler}\n\n" + "#### Sub\n\n" + "## Intro\n" + ) + f = tmp_path / "warnonly.md" + f.write_text(content, encoding="utf-8") + rc = cmd_validate_toc([str(f), "--max-level", "2"]) + out = json.loads(capsys.readouterr().out) + assert out["status"] == "WARN" + assert out["warning_count"] >= 3 + assert out["error_count"] == 0 + assert rc == 0 + class TestCmdTocValidation: """cmd_toc post-validation and error-status paths.""" diff --git a/vulture_whitelist.py b/vulture_whitelist.py index 056beb84..ecf54484 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -12,6 +12,7 @@ from studio.commands.kit import _read_conf_version from studio.commands.resolve_vars import assemble_component from studio.utils.context import LoadedKit +from studio.utils.doc_index import annotate_section_summary from studio.utils.eval_harness import ReferencePresenceScorer, Scenario, ScorerKind, run_suite from studio.utils.eval_judge import Gold from studio.utils.manifest import ManifestLayerState @@ -39,6 +40,12 @@ _ = Gold.rules_assessed # part of the gold format; consumed by per-rule judge scoring (future) INCLUDE_ERROR = ManifestLayerState.INCLUDE_ERROR # valid enum value for future use +# doc-index summary annotation: written by an LLM caller during a one-time +# enrichment pass over a cached index's sections; not yet reached from +# production paths. Exercised by tests. See +# skills/studio/scripts/studio/utils/doc_index.py. +annotate_section_summary # noqa: B018 + # cfs map module — symbols retained for layout/configuration completeness. from studio.commands.map.layout import MAX_ROW_W # noqa: E402 from studio.commands.map.categorize import OverrideCategory # noqa: E402