diff --git a/architecture/features/developer-experience.md b/architecture/features/developer-experience.md index 560db225..07e37daf 100644 --- a/architecture/features/developer-experience.md +++ b/architecture/features/developer-experience.md @@ -19,6 +19,7 @@ - [Run Self-Check](#run-self-check) - [Resolve Variables](#resolve-variables-1) - [Pylint Rollout Phase 0](#pylint-rollout-phase-0) + - [Change Summary Window And Events](#change-summary-window-and-events) - [4. States (CDSL)](#4-states-cdsl) - [Developer Experience State](#developer-experience-state) - [5. Definitions of Done](#5-definitions-of-done) @@ -230,6 +231,31 @@ Reduces friction in daily Studio usage. `doctor` catches environment issues befo 3. - `p2` - Keep the remaining backlog deferred for later rollout phases, starting with `R0917`, `R0902`, `C0302`, `C0415`, `R0401`, and `C0301` - `inst-pylint-phase-0-deferred-half` 4. - `p2` - Keep the rollout aligned with `cpt-studio-nfr-zero-harm`: stage advisory cleanup before enabling additional checks - `inst-pylint-phase-0-zero-harm` +### Change Summary Window And Events + +- [x] `p1` - **ID**: `cpt-studio-algo-developer-experience-change-summary` + +**Input**: A project root, plus an optional base ref or explicit lower-bound timestamp + +**Output**: The span of work a change digest covers, and the decision-log events recorded inside it + +**Rules**: +1. [x] - `p1` - Define the window and selection result types as immutable records, and the reason vocabulary shared by producer and renderer so an unavailable dimension is always named rather than shown as empty - `inst-change-summary-datamodel` +2. [x] - `p1` - Answer read-only git queries as one line of output or nothing, keeping a tool failure apart from a valid negative so a timeout is never reported as a conclusion about history - `inst-change-summary-git-query` +3. [x] - `p1` - Detect whether the project root sits inside a git work tree, telling not-a-repository apart from a repository without a working tree and from git itself failing to answer - `inst-change-summary-detect-repo` +4. [x] - `p1` - Resolve the base ref, preferring the canonical remote over a fork's lagging default, and honour or refuse an explicitly requested ref rather than substituting a fallback - `inst-change-summary-default-base` +5. [x] - `p1` - Resolve the merge-base between HEAD and the base ref, treating unrelated histories as no window - `inst-change-summary-merge-base` +6. [x] - `p1` - Read the base commit's commit time as the window's lower bound, accepting that the boundary moves with the merge-base and that an explicit lower bound is how a caller pins it - `inst-change-summary-base-time` +7. [x] - `p1` - Assemble the window, short-circuiting git when the caller supplies an explicit lower bound, and returning a stated reason on every failure path instead of raising - `inst-change-summary-resolve-window` +8. [x] - `p1` - Parse ISO-8601 timestamps to aware datetimes, normalising a trailing Z and refusing naive values rather than assuming an offset that would move events across the boundary - `inst-change-summary-parse-ts` +9. [x] - `p1` - Read the decision log once and take readability, the events and the corruption count from that single snapshot, keeping absent apart from unreadable, so nothing appended or rotated between separate reads is reported as this window's state - `inst-change-summary-log-state` +10. [x] - `p1` - Select events at or after the window boundary, excluding and counting undated events rather than guessing them into or out of the window - `inst-change-summary-select-events` +11. [x] - `p1` - Group selected events by run id in first-seen order, so one invocation is a subdivision of the branch's span and never the whole story - `inst-change-summary-group-runs` +12. [x] - `p1` - Resolve the decision log belonging to the window's own project rather than to the current working directory, following a process-wide override where the environment sets one but reporting that it did, so a digest never presents another project's decisions as this one's - `inst-change-summary-default-log` +13. [x] - `p1` - Walk a known-good base ref down to a window, letting a git tool failure take precedence over a historical reading and keeping whatever was already learned on the returned window - `inst-change-summary-window-from-base` +14. [x] - `p1` - Reduce a run id to a canonical form, casefolding and stripping so one logical run is not split and a non-string does not merge with its own text, while not rejecting an unrecognised-but-real identifier - `inst-change-summary-canonical-run` +15. [x] - `p1` - Resolve the decision log a window should be read from, returning either a usable path and whether the environment chose it, or the reason no log is usable - `inst-change-summary-resolve-log` + ## 4. States (CDSL) ### Developer Experience State @@ -289,6 +315,7 @@ No feature-specific state machines. Self-check is stateless (run → report). | TOC Command | `skills/.../commands/toc.py` | CLI wrapper for TOC generation | | TOC Utils | `skills/.../utils/toc.py` | Unified TOC generation, anchor slugs, code block awareness | | Resolve Vars Command | `skills/.../commands/resolve_vars.py` | Template variable resolution to absolute paths | +| Change Summary Core | `skills/.../utils/change_summary.py` | Window resolution from git, and decision-log event selection inside it | ## 7. Acceptance Criteria diff --git a/skills/studio/scripts/studio/utils/change_summary.py b/skills/studio/scripts/studio/utils/change_summary.py new file mode 100644 index 00000000..8dcbd680 --- /dev/null +++ b/skills/studio/scripts/studio/utils/change_summary.py @@ -0,0 +1,634 @@ +"""Change-summary core — resolve the window a digest covers, and select the +decision-log events recorded inside it. + +The digest answers "what changed on this branch, and why". This module owns the two +halves that have no output format: **which span of work counts as "the run"**, and +**which recorded decisions fall inside it**. Rendering belongs to the command +wrapper; linking changed files to requirements is separate again. + +Three deliberate choices: + +* **The window comes from git, not from a decision-log ``run_id``.** A ``run_id`` is + one CLI invocation, but a reviewer's "run" is a branch's worth of work. The window + is the span since the merge-base with the default branch, so ``run_id`` becomes a + grouping key *inside* that span rather than the span itself. The boundary is the + merge-base's own commit time, so it moves when the merge-base does — a rebase onto + newer upstream commits advances it, and decisions logged before the new base commit + fall outside the window. Git keeps no record of where a branch *used* to start, so + that is documented (see :func:`resolve_window`) rather than guessed around, and an + explicit ``since`` pins the boundary where the caller says. +* **Nothing here raises, and nothing here is silent.** Every path returns a value + carrying an explicit ``reason`` when a dimension is unavailable. A digest that + quietly shows less is the defect this effort exists to remove, so "cannot tell" is + always reported rather than rounded down to "nothing to say". +* **Reason strings carry no filesystem paths**, so no ``$HOME`` or username can + reach a rendered digest through them. + +Git access is a narrow read-only query helper, not a general runner. The two existing +private ``_run_git`` helpers in this package have incompatible contracts — one returns +``(code, stdout, stderr)``, the other returns a string and raises — so a third generic +copy would duplicate both. ``_git_query`` answers only "one line of stdout, or nothing — +and whether git itself failed to answer". + +@cpt-algo:cpt-studio-algo-developer-experience-change-summary:p1 +""" + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-datamodel +from __future__ import annotations + +import logging +import os +import subprocess +from dataclasses import dataclass +from datetime import datetime +from pathlib import Path +from typing import Any, Dict, List, Optional, Tuple + +from . import decision_log + +logger = logging.getLogger(__name__) + +#: Seconds any single git query may take before it is treated as unavailable. +_GIT_TIMEOUT = 10 + +#: Environment variables that redirect git away from the repository it was pointed at. +#: +#: ``cwd=`` is *not* sufficient on its own: an ambient ``GIT_DIR`` overrides it, so a +#: query about project A answered from project B's repository. Verified — +#: ``GIT_DIR=b/.git git -C a log`` reports b's commit, not a's. Every one of these is +#: cleared so the answer describes the project the caller named and nothing else. +_GIT_REDIRECT_VARS = ( + "GIT_DIR", + "GIT_WORK_TREE", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_COMMON_DIR", + "GIT_CEILING_DIRECTORIES", +) + +#: Refs tried in order when the caller names no base. +#: +#: ``upstream/*`` comes first deliberately. In a fork-based workflow — which this +#: project mandates, with ``origin`` pointing at the contributor's fork — ``origin/HEAD`` +#: tracks the *fork's* default branch, which lags the canonical one. Measured on this +#: checkout, ``origin/HEAD`` was five weeks behind ``upstream/main``, so preferring it +#: would silently widen every window to include work that shipped long ago. +#: A fresh clone of the canonical repo has no ``upstream`` remote, so it falls through +#: to ``origin/HEAD`` and is still correct. +#: ``upstream/HEAD`` leads because it is the canonical remote's *own* symbolic default +#: — right even when that default is neither ``main`` nor ``master``. Guessing branch +#: names first would skip it and fall through to a stale fork ref, which is the same +#: failure this ordering exists to prevent, one level deeper. +_DEFAULT_BASE_REFS = ( + "upstream/HEAD", + "upstream/main", + "upstream/master", + "origin/HEAD", + "origin/main", + "main", + "origin/master", + "master", +) + +# Reasons are module constants so the renderer and the tests share one vocabulary +# instead of matching on prose that can drift. +REASON_OK = "" +REASON_NOT_A_REPO = "not a git repository" +#: A bare repository, or a path inside the ``.git`` directory itself. Both *are* +#: repositories, so reporting them as :data:`REASON_NOT_A_REPO` was a false statement. +REASON_NO_WORK_TREE = "in a git repository but outside any working tree" +REASON_GIT_UNAVAILABLE = "git unavailable" +#: Kept free of an enumerated candidate list on purpose: the first version of this +#: string named the refs it tried, and went stale the moment the list changed. +REASON_NO_BASE_REF = "no default base ref found" +REASON_BASE_REF_UNKNOWN = "requested base ref not found" +REASON_NO_MERGE_BASE = "no merge base with the base ref" +REASON_NO_BASE_TIME = "base commit has no readable timestamp" +REASON_NOT_A_PROJECT = "not inside a Studio project" +REASON_LOG_DISABLED = "decision log disabled" +REASON_LOG_ABSENT = "no decision log yet" +REASON_LOG_UNREADABLE = "decision log unreadable" +REASON_INVALID_SINCE = "the supplied lower bound is not an absolute timestamp" + +#: Bucket name for selected events whose ``run_id`` is missing or unusable. Grouping +#: used to build buckets only for truthy ids, so such events sat in ``events`` and in no +#: group, and a renderer summing the groups under-reported without saying so. +#: +#: The parentheses make it read as a label rather than an identifier, but nothing +#: *enforces* uniqueness: a writer emitting this exact string would share the bucket. +#: That is a deliberate trade — the alternative is rejecting unrecognised ids, which +#: discards real information (see :func:`_canonical_run_id`). Sharing a label is +#: cosmetic; dropping an event is not. +RUN_UNATTRIBUTED = "(unattributed)" + + +@dataclass(frozen=True) +class ChangeWindow: + """The span of work a digest covers. + + ``available`` false means no git-derived window could be established; ``reason`` + then says which of the failure modes applied. ``since`` is the base commit's own + commit time, which is what makes the window "everything after the branch point". + + Frozen: a window records what git said at one moment, and nothing here mutates one + after construction, so nothing may. + """ + + project_root: str = "" + base_ref: str = "" + base_sha: str = "" + since: str = "" + available: bool = False + reason: str = REASON_NOT_A_REPO + + +@dataclass(frozen=True) +class EventSelection: + """Decision-log events falling inside a window. + + ``skipped_lines`` is the number of non-empty log lines that yielded no event — not + JSON, or JSON that is not an object. It is exact for the snapshot the selection was + read from, because the events and the line count come from one read of the file + (see :func:`_read_log`), and it is reported rather than hidden. + + ``log_overridden`` says the environment named the log (``$CFS_DECISION_LOG``) rather + than the window's project. That log is shared by every project the process ran in, + so its events cannot be attributed to this window's project, and a digest should + say so instead of presenting them as the project's own. + + Frozen, with ``events`` and ``runs`` as tuples: the counts describe those + collections, and a caller able to grow or shrink them would silently make the + counts wrong. + """ + + events: Tuple[Dict[str, Any], ...] = () + runs: Tuple[str, ...] = () + scanned: int = 0 + undated: int = 0 + runless: int = 0 + skipped_lines: int = 0 + log_overridden: bool = False + available: bool = False + reason: str = REASON_NOT_A_PROJECT +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-datamodel + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-git-query +def _git_env() -> Dict[str, str]: + """The ambient environment with git's repository-redirecting variables removed.""" + env = dict(os.environ) + for name in _GIT_REDIRECT_VARS: + env.pop(name, None) + return env + + +def _git_query(project_root: Path, args: List[str]) -> Tuple[Optional[str], bool]: + """Run a read-only git query, returning ``(first line or None, tool_failed)``. + + The two halves of "no answer" are kept apart, because conflating them lets a + transient tool failure be reported as a conclusion about history — "no merge base" + when git simply timed out. + + * **Tool failure** is git not launching, or timing out. Nothing was learned. + * **A non-zero exit is a valid negative**, not a failure: ``merge-base`` exits 1 + when two histories genuinely have no common ancestor, and + ``rev-parse --verify --quiet`` exits 1 when a ref genuinely does not exist. Those + are answers, and treating them as breakage would be just as misleading in the + other direction. + + Never raises. + """ + try: + result = subprocess.run( + ["git"] + args, + cwd=str(project_root), + env=_git_env(), + capture_output=True, + text=True, + # Git refs and paths are bytes and need not be UTF-8, so `text=True`'s + # strict default would raise UnicodeDecodeError past the handler below and + # break the never-raises contract. `surrogateescape` is the handler Python + # uses for filesystem values, so they round-trip to the same bytes. + errors="surrogateescape", + timeout=_GIT_TIMEOUT, + check=False, + ) + except (OSError, UnicodeDecodeError, subprocess.SubprocessError) as exc: + logger.debug("change-summary git query could not run: %s", exc) + return None, True + if result.returncode: + logger.debug("change-summary git query exited %d", result.returncode) + return None, False + line = result.stdout.strip().splitlines() + return (line[0].strip() if line else None), False +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-git-query + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-detect-repo +def _detect_repo(project_root: Path) -> str: + """Return :data:`REASON_OK` when ``project_root`` is inside a git work tree, else why not. + + One query, three answers, kept apart: + + * a **tool failure** is :data:`REASON_GIT_UNAVAILABLE`. Nothing was learned, so + nothing is claimed about the directory. An earlier version dropped this flag and + asked ``git --version`` separately to guess between "not a repo" and "git broken" + — so a timeout here followed by a healthy second launch was reported as a fact + about the directory that was never established, and every non-repo path cost two + launches instead of one; + * a **non-zero exit** is :data:`REASON_NOT_A_REPO`: git looked, and there is none; + * ``false`` is :data:`REASON_NO_WORK_TREE`: a bare repository, or the ``.git`` + directory itself. Both are repositories, and a digest of working-tree changes + needs a working tree, so the reason names the thing that is actually missing. + """ + answer, failed = _git_query(project_root, ["rev-parse", "--is-inside-work-tree"]) + if failed: + return REASON_GIT_UNAVAILABLE + if answer is None: + return REASON_NOT_A_REPO + return REASON_OK if answer == "true" else REASON_NO_WORK_TREE +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-detect-repo + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-default-base +def _resolve_base_ref(project_root: Path, requested: str = "") -> Tuple[Optional[str], bool]: + """Pick the ref the window is measured from. + + An explicitly requested ref is honoured or refused — never silently swapped for a + fallback, because a digest measured against a different ref than the caller asked + for is worse than one that says it could not comply. + """ + if requested: + if "\0" in requested: + # No ref can contain NUL, and `subprocess` refuses to pass one — raising + # ValueError before git starts, outside `_git_query`'s handler. Refuse it + # here as what it is: a ref that cannot exist. + return None, False + resolved, failed = _git_query( + project_root, + # `--end-of-options` so a caller-supplied ref beginning with a dash is read + # as a ref rather than as a git option. + ["rev-parse", "--verify", "--quiet", "--end-of-options", requested], + ) + return (requested if resolved else None), failed + for candidate in _DEFAULT_BASE_REFS: + resolved, failed = _git_query( + project_root, ["rev-parse", "--verify", "--quiet", "--end-of-options", candidate], + ) + if failed: + # Stop at the first tool failure rather than walking the remaining + # candidates: each would fail the same way, and reporting "no default base + # ref" after eight failed launches states a fact about the repository that + # was never established. + return None, True + if resolved: + return candidate, False + return None, False +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-default-base + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-merge-base +def _merge_base(project_root: Path, base_ref: str) -> Tuple[Optional[str], bool]: + """Return the merge-base sha between ``HEAD`` and ``base_ref``. + + Returns ``(sha, tool_failed)``. Unrelated histories and a missing ref yield a + ``None`` sha with ``tool_failed`` false — there is genuinely no branch point. A + ``True`` flag means git never answered, which is a different fact and must not be + reported as a finding about history. + """ + return _git_query(project_root, ["merge-base", "--end-of-options", "HEAD", base_ref]) +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-merge-base + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-base-time +def _commit_time(project_root: Path, sha: str) -> Tuple[Optional[str], bool]: + """Commit time in strict ISO 8601, plus whether git itself failed.""" + return _git_query(project_root, ["show", "-s", "--format=%cI", "--end-of-options", sha]) +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-base-time + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-resolve-window +def resolve_window( + project_root: Path, + *, + base: str = "", + since: str = "", +) -> ChangeWindow: + """Resolve the span of work a digest should cover. + + ``since`` short-circuits git entirely — an explicit lower bound is the caller's + assertion and needs no branch point. Otherwise the window starts at the merge-base + with ``base`` (or the first of :data:`_DEFAULT_BASE_REFS` that exists). + + **The boundary moves with the merge-base.** It is the base commit's *commit* time, + so rebasing onto newer upstream commits advances it, and decisions logged before + the new base commit was committed then fall outside the window — while the files + they concern still show as changed, because file changes are measured by tree + content, not by time. Git keeps no record of where the branch used to start, so + the old boundary cannot be recovered from history. The sha and time that *were* + used are carried on the window so a digest can print them, and ``since`` pins the + boundary where the caller says. Widening automatically to the branch's earliest + author date was considered and rejected: author dates are arbitrary (cherry-picks, + ``--date``), so one old commit would silently pull years of unrelated decisions + into the window — the opposite failure, and harder to notice. + + Every failure returns an unavailable window carrying its reason. Never raises. + """ + # Resolved, not merely stored. A relative root left as-is puts the cwd dependence + # straight back: `resolve_window(Path("."))` recorded "." and a later chdir then + # redirected log resolution to a different project — the exact failure carrying the + # root on the window was added to prevent. Resolving also fixes the git cwd, since + # `subprocess(cwd=...)` resolves a relative path at call time, not at capture time. + project_root = Path(project_root).resolve() + root = str(project_root) + if since: + # A caller-supplied bound is validated here rather than surfacing later as a + # complaint about a base commit that was never consulted. + if _parse_ts(since) is None: + return ChangeWindow(project_root=root, reason=REASON_INVALID_SINCE) + return ChangeWindow(project_root=root, since=since, available=True, reason=REASON_OK) + + repo_state = _detect_repo(project_root) + if repo_state: + return ChangeWindow(project_root=root, reason=repo_state) + + base_ref, failed = _resolve_base_ref(project_root, base) + if failed: + return ChangeWindow(project_root=root, reason=REASON_GIT_UNAVAILABLE) + if base_ref is None: + # Two different failures, two different reasons: a ref the caller named and + # git does not have, versus no discoverable default at all. + return ChangeWindow( + project_root=root, + reason=REASON_BASE_REF_UNKNOWN if base else REASON_NO_BASE_REF, + ) + return _window_from_base_ref(project_root, base_ref) +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-resolve-window + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-window-from-base +def _window_from_base_ref(project_root: Path, base_ref: str) -> ChangeWindow: + """Walk a known-good base ref down to a window, or the reason it could not. + + Split out of :func:`resolve_window` to keep each function's guard clauses legible; + the whole point of this stage is that there are several distinct ways to fail and + each gets its own reported reason rather than a shared shrug. + + Past this point git has already answered once, so a further non-answer is + ambiguous: it may be a genuine negative about history, or the tool falling over. + Reporting "no merge base" for a timeout would be a false conclusion, so the failure + flag takes precedence over the historical reading. Whatever *was* learned — the ref, + then the sha — stays on the returned window so a reason keeps its subject. + """ + root = str(project_root) + base_sha, failed = _merge_base(project_root, base_ref) + if failed: + return ChangeWindow(project_root=root, base_ref=base_ref, reason=REASON_GIT_UNAVAILABLE) + if base_sha is None: + return ChangeWindow(project_root=root, base_ref=base_ref, reason=REASON_NO_MERGE_BASE) + + base_time, failed = _commit_time(project_root, base_sha) + if failed or base_time is None: + return ChangeWindow( + project_root=root, base_ref=base_ref, base_sha=base_sha, + reason=REASON_GIT_UNAVAILABLE if failed else REASON_NO_BASE_TIME, + ) + + return ChangeWindow( + project_root=root, + base_ref=base_ref, + base_sha=base_sha, + since=base_time, + available=True, + reason=REASON_OK, + ) +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-window-from-base + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-parse-ts +def _parse_ts(value: Any) -> Optional[datetime]: + """Parse an ISO-8601 timestamp to an aware ``datetime``, or ``None``. + + A trailing ``Z`` is normalised because git and the log writer disagree about it. + A naive timestamp is refused rather than assumed to be UTC: guessing an offset + would silently move events across the window boundary. + """ + if not isinstance(value, str) or not value: + return None + text = value[:-1] + "+00:00" if value.endswith("Z") else value + try: + parsed = datetime.fromisoformat(text) + except ValueError as exc: + logger.debug("change-summary could not parse timestamp %r: %s", value, exc) + return None + return parsed if parsed.tzinfo is not None else None +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-parse-ts + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-log-state +def _read_log(path: Path) -> Tuple[Optional[List[str]], str]: + """Read the whole log once, returning ``(non-empty lines, reason)``. + + One open, one decode, one snapshot. Readability, the events and the line count the + corruption figure is derived from all come out of this single read, so nothing can + change underneath the selection: + + * an earlier design probed readability, let :func:`decision_log.read_events` open + the file again, then opened it a third time to count lines. A writer appending a + valid line between the last two opens made the count exceed the events, so normal + concurrent activity was reported as corruption; a rotation between the first two + swapped the verified file for a fresh near-empty one, and the selection came back + clean and almost empty with no sign that anything had moved; + * readability is proved by reading, not inferred from ``stat``: ``is_file`` passes + a mode-000 file, and only a strict decode catches bytes that are not UTF-8. + + Absent and unreadable stay distinct — "no decision log yet" and "the log could not + be read" call for different actions. Never raises. + """ + try: + text = path.read_text(encoding="utf-8") + except (FileNotFoundError, NotADirectoryError) as exc: + logger.debug("change-summary found no decision log: %s", exc) + return None, REASON_LOG_ABSENT + except (OSError, UnicodeDecodeError) as exc: + logger.debug("change-summary log is unreadable: %s", exc) + return None, REASON_LOG_UNREADABLE + return [line for line in text.splitlines() if line.strip()], REASON_OK +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-log-state + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-default-log +def _default_log_for(window: ChangeWindow) -> Tuple[Optional[Path], bool]: + """Resolve the decision log for *the window's* project, and whether the environment chose it. + + ``decision_log.default_log_path()`` defaults to the cwd, which is correct for the + writer — it logs whichever project the command runs in. A reader reporting on an + explicitly named project must not inherit that default, or the digest describes one + project's changes alongside another project's decisions. + + ``$CFS_DECISION_LOG`` names one log for the whole process, and the writer honours it + in every project the process runs in — so the reader follows it too, because that + is where the events *are*. Reading the project-local path instead would report "no + decision log yet" about a log that exists and is being written to. What the reader + cannot do is attribute a shared log's events to this window's project, so the + second value says the environment chose the log, and the selection carries that + rather than presenting a shared log as the project's own. + """ + overridden = decision_log.override_log_path() is not None + if not window.project_root: + return decision_log.default_log_path(), overridden + return decision_log.default_log_path(Path(window.project_root)), overridden +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-default-log + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-resolve-log +def _resolve_log_for( + window: ChangeWindow, path: Optional[Path], +) -> Tuple[Optional[Path], str, bool]: + """Return ``(log path, reason, overridden)`` — the path and the reason are exclusive. + + Split out of :func:`select_events` so each function's guard clauses stay within the + project's return-count budget, and so "which log, and is it this project's own" is + answerable on its own. Whether the log can be *read* is answered by reading it — + :func:`_read_log` — not by a separate probe the file could change after. + """ + if not decision_log.is_enabled(): + return None, REASON_LOG_DISABLED, False + if path is not None: + return path, REASON_OK, False + target, overridden = _default_log_for(window) + if target is None: + return None, REASON_NOT_A_PROJECT, False + return target, REASON_OK, overridden +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-resolve-log + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-select-events +def select_events( + window: ChangeWindow, + *, + path: Optional[Path] = None, +) -> EventSelection: + """Select the decision-log events recorded inside ``window``. + + An event whose timestamp cannot be parsed is **excluded and counted** in + ``undated`` rather than guessed into or out of the window — the caller can then + say so instead of presenting a quietly incomplete list. + + An unavailable window yields an unavailable selection carrying the window's own + reason, so the caller reports one cause rather than two. + + The default log is resolved from **the window's own project**, not from the current + working directory. Those are independent inputs, so a window built for project A + while the process sits in project B used to select B's decisions — a digest about + one project carrying another's history. + + The log is read **once**, and everything reported comes from that one snapshot — + see :func:`_read_log` for the two races that separate reads allowed. Never raises. + """ + if not window.available: + return EventSelection(reason=window.reason) + + target, reason, overridden = _resolve_log_for(window, path) + if target is None: + return EventSelection(reason=reason) + + boundary = _parse_ts(window.since) + if boundary is None: + return EventSelection(reason=REASON_NO_BASE_TIME) + + lines, reason = _read_log(target) + if lines is None: + return EventSelection(reason=reason) + + selected, runs, scanned, undated, runless = [], [], 0, 0, 0 + for event in decision_log.parse_events(lines): + scanned += 1 + stamp = _parse_ts(event.get("ts")) + if stamp is None: + undated += 1 + continue + if stamp < boundary: + continue + selected.append(event) + run_id = _canonical_run_id(event.get("run_id")) + if not run_id: + runless += 1 + run_id = RUN_UNATTRIBUTED + if run_id not in runs: + runs.append(run_id) + + return EventSelection( + events=tuple(selected), + runs=tuple(runs), + scanned=scanned, + undated=undated, + runless=runless, + # Exact, not a bound: the lines and the events came from the same snapshot. + skipped_lines=len(lines) - scanned, + log_overridden=overridden, + available=True, + reason=REASON_OK, + ) +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-select-events + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-canonical-run +def _canonical_run_id(value: Any) -> str: + """Return the canonical form of a ``run_id``, or ``""`` when it is not usable. + + Raw values were used directly as grouping keys, which fragmented and merged history + in three ways: + + * **case variants split one run** — ``"AB12"`` and ``"ab12"`` became two groups for + something the writer would only ever have emitted once, so casefolding merges them; + * **a non-string merged with its own text** — ``1`` and ``"1"`` both stringified to + ``"1"``, so a numeric field silently joined an unrelated run. Only ``str`` is + accepted, which keeps them apart; + * **whitespace formed an attributed group** — ``" "`` is truthy, so it looked like + a real run. Stripping sends it to :data:`RUN_UNATTRIBUTED` where it belongs. + + What this deliberately does **not** do is require the writer's current shape + (``uuid4().hex[:12]``). Rejecting anything non-hexadecimal would discard a real, + distinguishing identifier by folding it into the anonymous bucket, and + ``decision_log``'s own schema is explicit that "readers must ignore unknown event + names and unknown payload keys so that newer instrumentation never breaks an older + reader". A reader that refuses a run id it does not recognise breaks exactly that. + An unrecognised-but-present id is more honestly reported under its own name than + merged into "unattributed", which is a claim that no id was recorded at all. + + ``casefold`` rather than ``lower``: the writer's own ids are ASCII hex, where the + two agree, but the contract is casefolding — and ``lower`` leaves ``"Straße"`` and + ``"STRASSE"`` as two runs. + """ + if not isinstance(value, str): + return "" + return value.strip().casefold() +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-canonical-run + + +# @cpt-begin:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-group-runs +def group_by_run(selection: EventSelection) -> Dict[str, List[Dict[str, Any]]]: + """Group a selection's events by ``run_id``, preserving first-seen run order. + + This is the role ``run_id`` keeps once the window stops being derived from it: a + subdivision *within* the branch's span, so a digest can say "three invocations" + without treating the last one as the whole story. + + **Every selected event lands in exactly one bucket.** Events carrying a blank or + missing ``run_id`` go to :data:`RUN_UNATTRIBUTED`; previously buckets were built + only for truthy ids, so such an event sat in ``events`` and in no group at all and + a renderer summing the groups under-reported without saying so. + + The buckets hold the selection's own event objects, not copies. One event has one + identity: a copy would let a renderer annotate a group and then read a different + value back from ``events`` — two truths where there was one. The selection itself is + frozen and its collections are tuples, so this view cannot make its counts wrong. + """ + grouped: Dict[str, List[Dict[str, Any]]] = {run: [] for run in selection.runs} + for event in selection.events: + run_id = _canonical_run_id(event.get("run_id")) or RUN_UNATTRIBUTED + grouped.setdefault(run_id, []).append(event) + return grouped +# @cpt-end:cpt-studio-algo-developer-experience-change-summary:p1:inst-change-summary-group-runs diff --git a/skills/studio/scripts/studio/utils/decision_log.py b/skills/studio/scripts/studio/utils/decision_log.py index 40302695..383e8b07 100644 --- a/skills/studio/scripts/studio/utils/decision_log.py +++ b/skills/studio/scripts/studio/utils/decision_log.py @@ -45,7 +45,7 @@ import uuid from datetime import datetime, timezone from pathlib import Path -from typing import Any, Dict, Iterator, List, Optional +from typing import Any, Dict, Iterable, Iterator, List, Optional logger = logging.getLogger(__name__) @@ -113,21 +113,41 @@ def opt_out_sentinel_path() -> Path: return _brand_dir() / _OPT_OUT_SENTINEL -def default_log_path() -> Optional[Path]: +def override_log_path() -> Optional[Path]: + """Return the log named by ``$CFS_DECISION_LOG``, or ``None`` when unset or an off-value. + + The override is process-wide: the writer honours it in every project the process + runs in, so a reader that wants to read what the writer wrote must honour it too — + and may want to know that it did, since one shared log cannot be attributed to any + single project. + """ + override = os.environ.get(_ENV_PATH, "").strip() + if override and override.lower() not in _OFF_VALUES: + return Path(override).expanduser() + return None + + +def default_log_path(start: Optional[Path] = None) -> Optional[Path]: """Resolve the log location, or ``None`` when there is nowhere to write. Order: 1. ``$CFS_DECISION_LOG`` if it names a path (an off-value there disables logging). - 2. ``/.cache/decisions.jsonl`` for the project containing the cwd. + 2. ``/.cache/decisions.jsonl`` for the project containing ``start``. 3. ``None`` — outside a project, so the writer no-ops. + + ``start`` defaults to the cwd, which is right for the writer: it logs whatever + project the command is running in. A *reader* working against an explicitly named + project must pass that root, or it can resolve a different project's log than the + one it is reporting on. The override wins over ``start`` deliberately — see + :func:`override_log_path`. """ - override = os.environ.get(_ENV_PATH, "").strip() - if override and override.lower() not in _OFF_VALUES: - return Path(override).expanduser() + override = override_log_path() + if override is not None: + return override try: from .files import find_studio_directory - studio_dir = find_studio_directory(Path.cwd()) + studio_dir = find_studio_directory(start or Path.cwd()) except Exception: # pylint: disable=broad-except studio_dir = None if studio_dir is None: @@ -380,6 +400,28 @@ def record_read(method: str, target: str, lines: int, tokens: int, source: str = # Reading # --------------------------------------------------------------------------- # @cpt-begin:cpt-studio-algo-core-infra-decision-log:p1:inst-log-read +def parse_events(lines: Iterable[str]) -> Iterator[Dict[str, Any]]: + """Yield the event objects among ``lines``, skipping any line that will not parse. + + The parsing rules of :func:`read_events`, on their own. A reader that has taken its + own snapshot of the file — to count lines and select events from the *same* bytes, + so nothing appended between two reads can be mistaken for corruption — parses that + snapshot the way this module does, rather than growing a second copy of the rules. + """ + for line in lines: + line = line.strip() + if not line: + continue + try: + obj = json.loads(line) + except (ValueError, TypeError) as exc: + logger.debug("decision log: skipping unparseable line: %s", exc) + continue + if not isinstance(obj, dict): + continue + yield obj + + def read_events(path: Optional[Path] = None, *, event: str = "", run_id: str = "", decision_id: str = "", limit: int = 0) -> Iterator[Dict[str, Any]]: @@ -401,17 +443,7 @@ def read_events(path: Optional[Path] = None, *, event: str = "", return matched: List[Dict[str, Any]] = [] - for line in lines: - line = line.strip() - if not line: - continue - try: - obj = json.loads(line) - except (ValueError, TypeError) as exc: - logger.debug("decision log: skipping unparseable line: %s", exc) - continue - if not isinstance(obj, dict): - continue + for obj in parse_events(lines): if event and obj.get("event") != event: continue if run_id and obj.get("run_id") != run_id: diff --git a/tests/test_change_summary_core.py b/tests/test_change_summary_core.py new file mode 100644 index 00000000..bc9c76c4 --- /dev/null +++ b/tests/test_change_summary_core.py @@ -0,0 +1,1299 @@ +"""Tests for the change-summary core — window resolution and event selection. + +The module's whole contract is *never raise, never go silent*: every failure path +returns a value carrying a reason. So these tests are weighted toward forcing each +failure rather than confirming the happy path, and several assert on the *reason* +rather than merely on emptiness — an empty result with the wrong explanation is the +defect this feature exists to remove. +""" + +from __future__ import annotations + +import dataclasses +import json +import os +import socket +import subprocess +from pathlib import Path + +import pytest + +from studio.utils import change_summary as cs +from studio.utils import decision_log + + +# --------------------------------------------------------------------------- helpers + +def _git(repo: Path, *args: str) -> str: + result = subprocess.run( + ["git", *args], cwd=str(repo), capture_output=True, text=True, check=True, + ) + return result.stdout.strip() + + +def _make_repo(repo: Path) -> Path: + """A minimal repo with one commit and deterministic identity.""" + repo.mkdir(parents=True, exist_ok=True) + _git(repo, "init", "-q") + _git(repo, "config", "user.email", "test@example.com") + _git(repo, "config", "user.name", "Test User") + _git(repo, "config", "commit.gpgsign", "false") + (repo / "a.txt").write_text("one\n", encoding="utf-8") + _git(repo, "add", "a.txt") + _git(repo, "commit", "-q", "-m", "initial") + return repo + + +def _commit(repo: Path, name: str, body: str = "x\n") -> str: + (repo / name).write_text(body, encoding="utf-8") + _git(repo, "add", name) + _git(repo, "commit", "-q", "-m", f"add {name}") + return _git(repo, "rev-parse", "HEAD") + + +def _point_ref(repo: Path, ref: str, sha: str) -> None: + """Create a remote-tracking ref without needing a real remote.""" + _git(repo, "update-ref", ref, sha) + + +def _make_studio_project(root: Path) -> Path: + """A directory the real project/studio resolvers recognise. + + `find_project_root` wants the `@cf:root-agents` marker in AGENTS.md, and + `find_studio_directory` then reads the `studio` key from its TOML block. + """ + (root / ".studio" / "rules").mkdir(parents=True) + (root / "AGENTS.md").write_text( + '\n\n```toml\nstudio = ".studio"\n```\n', encoding="utf-8", + ) + (root / ".studio" / "AGENTS.md").write_text("# studio\n", encoding="utf-8") + return root + + +def _write_log(path: Path, rows: list, extra: str = "") -> Path: + body = "".join(json.dumps(r) + "\n" for r in rows) + extra + path.write_text(body, encoding="utf-8") + return path + + +def _event(ts: str, run_id: str = "r1", event: str = "validation") -> dict: + return {"schema": 1, "ts": ts, "run_id": run_id, "decision_id": "d", + "event": event, "command": "validate", "payload": {}} + + +# --------------------------------------------------------------------------- window + +class TestTheWindowComesFromGit: + + def test_a_repo_with_a_base_ref_yields_a_window(self, tmp_path): + repo = _make_repo(tmp_path / "r") + base = _git(repo, "rev-parse", "HEAD") + _point_ref(repo, "refs/remotes/upstream/main", base) + _commit(repo, "b.txt") + + window = cs.resolve_window(repo) + + assert window.available is True + assert window.reason == cs.REASON_OK + assert window.base_ref == "upstream/main" + assert window.base_sha == base + assert window.since # the base commit's own time + + def test_an_explicit_since_short_circuits_git_entirely(self): + # A path that is not a repo, and does not exist: proof no git call is needed. + window = cs.resolve_window(Path("/nonexistent-abc"), since="2026-01-01T00:00:00+00:00") + + assert window.available is True + assert window.since == "2026-01-01T00:00:00+00:00" + assert window.base_ref == "" + + def test_an_explicit_base_ref_is_honoured(self, tmp_path): + repo = _make_repo(tmp_path / "r") + base = _git(repo, "rev-parse", "HEAD") + _git(repo, "branch", "release") + _commit(repo, "b.txt") + + window = cs.resolve_window(repo, base="release") + + assert window.base_ref == "release" + assert window.base_sha == base + + def test_an_unknown_explicit_base_ref_is_refused_not_swapped(self, tmp_path): + """The mutation that matters: a default ref exists, so a fallback would 'work'.""" + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + + window = cs.resolve_window(repo, base="no-such-ref") + + assert window.available is False + assert window.reason == cs.REASON_BASE_REF_UNKNOWN + # Must NOT have silently measured against the discoverable default. + assert window.base_ref == "" + + def test_the_canonical_remote_is_preferred_over_a_lagging_fork(self, tmp_path): + """In a fork workflow origin is the contributor's fork and lags upstream. + + Preferring origin/HEAD would widen the window to include long-shipped work. + """ + repo = _make_repo(tmp_path / "r") + old = _git(repo, "rev-parse", "HEAD") + newer = _commit(repo, "b.txt") + _commit(repo, "c.txt") + _point_ref(repo, "refs/remotes/origin/HEAD", old) # the stale fork + _point_ref(repo, "refs/remotes/upstream/main", newer) # the canonical remote + + window = cs.resolve_window(repo) + + assert window.base_ref == "upstream/main" + assert window.base_sha == newer + + def test_the_canonical_remotes_own_default_wins_even_when_unnamed(self, tmp_path): + """A remote whose default is neither main nor master, e.g. trunk. + + Guessing branch names first would skip the symbolic upstream/HEAD and fall + through to the stale fork ref — the same bug one level deeper. + """ + repo = _make_repo(tmp_path / "r") + old = _git(repo, "rev-parse", "HEAD") + newer = _commit(repo, "b.txt") + _commit(repo, "c.txt") + _point_ref(repo, "refs/remotes/origin/HEAD", old) # stale fork + _point_ref(repo, "refs/remotes/upstream/trunk", newer) # unconventional name + _git(repo, "symbolic-ref", "refs/remotes/upstream/HEAD", "refs/remotes/upstream/trunk") + + window = cs.resolve_window(repo) + + assert window.base_ref == "upstream/HEAD" + assert window.base_sha == newer, "must not fall through to the stale fork ref" + + +class TestTheWindowSaysWhyItCouldNotBeBuilt: + + def test_a_non_repo_is_reported_not_crashed(self, tmp_path): + window = cs.resolve_window(tmp_path) + + assert window.available is False + assert window.reason == cs.REASON_NOT_A_REPO + + def test_git_unavailable_is_distinguished_from_not_a_repo(self, tmp_path, monkeypatch): + monkeypatch.setattr(cs, "_git_query", lambda *_a, **_k: (None, True)) + + window = cs.resolve_window(tmp_path) + + assert window.reason == cs.REASON_GIT_UNAVAILABLE + + def test_no_discoverable_base_ref_is_reported(self, tmp_path): + repo = _make_repo(tmp_path / "r") + # Rename the only branch to something none of the candidates match. + _git(repo, "branch", "-m", "wip-nothing-standard") + + window = cs.resolve_window(repo) + + assert window.available is False + assert window.reason == cs.REASON_NO_BASE_REF + + def test_unrelated_histories_have_no_merge_base(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.setattr(cs, "_merge_base", lambda *_a, **_k: (None, False)) + + window = cs.resolve_window(repo) + + assert window.available is False + assert window.reason == cs.REASON_NO_MERGE_BASE + assert window.base_ref == "upstream/main" # what we did learn is still reported + + def test_a_base_commit_without_a_readable_time_is_reported(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.setattr(cs, "_commit_time", lambda *_a, **_k: (None, False)) + + window = cs.resolve_window(repo) + + assert window.reason == cs.REASON_NO_BASE_TIME + assert window.base_sha # still reported + + def test_the_reason_list_has_no_enumerated_ref_names(self): + """The first version of this string listed the refs it tried and went stale.""" + assert "origin" not in cs.REASON_NO_BASE_REF + assert "main" not in cs.REASON_NO_BASE_REF + + +# --------------------------------------------------------------------------- events + +class TestEventSelection: + + def test_events_before_the_boundary_are_excluded(self, tmp_path): + window = cs.ChangeWindow(since="2026-06-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-05-31T23:59:59+00:00", "old"), + _event("2026-06-02T00:00:00+00:00", "new"), + ]) + + selection = cs.select_events(window, path=log) + + assert selection.available is True + assert [e["run_id"] for e in selection.events] == ["new"] + assert selection.scanned == 2 + + def test_an_event_exactly_at_the_boundary_is_included(self, tmp_path): + boundary = "2026-06-01T00:00:00+00:00" + window = cs.ChangeWindow(since=boundary, available=True) + log = _write_log(tmp_path / "d.jsonl", [_event(boundary, "edge")]) + + selection = cs.select_events(window, path=log) + + assert len(selection.events) == 1, "the branch point itself belongs to the window" + + def test_a_z_suffix_and_an_offset_compare_correctly(self, tmp_path): + window = cs.ChangeWindow(since="2026-06-01T00:00:00Z", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T01:00:00+02:00", "before"), # 23:00 previous day UTC + _event("2026-06-01T03:00:00+02:00", "after"), # 01:00 UTC + ]) + + selection = cs.select_events(window, path=log) + + assert [e["run_id"] for e in selection.events] == ["after"] + + def test_unparseable_lines_are_counted_not_hidden(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log( + tmp_path / "d.jsonl", + [_event("2026-06-01T00:00:00+00:00")], + extra="{ this is not json\n\n[]\n", + ) + + selection = cs.select_events(window, path=log) + + assert selection.skipped_lines >= 1, "corruption must be reported, not swallowed" + + def test_undated_events_are_excluded_and_counted(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + rows = [ + _event("2026-06-01T00:00:00+00:00", "dated"), + {"schema": 1, "run_id": "no-ts", "event": "validation", "payload": {}}, + _event("not-a-timestamp", "bad-ts"), + ] + log = _write_log(tmp_path / "d.jsonl", rows) + + selection = cs.select_events(window, path=log) + + assert [e["run_id"] for e in selection.events] == ["dated"] + assert selection.undated == 2, "cannot-tell is counted, never guessed either way" + + def test_a_naive_timestamp_is_refused_rather_than_assumed_utc(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00", "naive")]) + + selection = cs.select_events(window, path=log) + + assert selection.events == () + assert selection.undated == 1 + + def test_runs_preserve_first_seen_order(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T00:00:01+00:00", "second"), + _event("2026-06-01T00:00:02+00:00", "first"), + _event("2026-06-01T00:00:03+00:00", "second"), + ]) + + selection = cs.select_events(window, path=log) + + assert selection.runs == ("second", "first"), "first-seen order, not sorted" + + def test_the_scanned_count_is_reported_even_when_nothing_is_selected(self, tmp_path): + window = cs.ChangeWindow(since="2026-12-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T00:00:00+00:00"), _event("2026-06-02T00:00:00+00:00"), + ]) + + selection = cs.select_events(window, path=log) + + assert selection.events == () + assert selection.scanned == 2, "a verdict without its denominator is the defect" + + +class TestEventSelectionSaysWhyItCouldNotRead: + + def test_an_unavailable_window_propagates_exactly_one_reason(self, tmp_path): + window = cs.resolve_window(tmp_path) # not a repo + + selection = cs.select_events(window) + + assert selection.available is False + assert selection.reason == window.reason, "one cause reported, not two" + + def test_a_disabled_log_is_reported(self, tmp_path, monkeypatch): + monkeypatch.setenv("CFS_DECISION_LOG", "0") + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=tmp_path / "d.jsonl") + + assert selection.reason == cs.REASON_LOG_DISABLED + + def test_an_absent_log_is_reported(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=tmp_path / "never-written.jsonl") + + assert selection.reason == cs.REASON_LOG_ABSENT + + def test_outside_a_studio_project_is_reported(self, monkeypatch): + monkeypatch.delenv("CFS_DECISION_LOG", raising=False) + monkeypatch.setattr(decision_log, "default_log_path", lambda: None) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window) + + assert selection.reason == cs.REASON_NOT_A_PROJECT + + def test_a_hand_built_window_with_an_unparseable_bound_still_degrades(self, tmp_path): + """`resolve_window` now rejects a bad `since` up front, so this can only be + reached by constructing a window directly. The guard stays as the floor beneath + that, but it is no longer the path a caller passing `since` takes — see + `TestAnExplicitSinceIsValidatedUpFront`, which is where that case belongs. + """ + window = cs.ChangeWindow(since="nonsense", available=True) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + + selection = cs.select_events(window, path=log) + + assert selection.available is False + assert selection.reason == cs.REASON_NO_BASE_TIME + + +# --------------------------------------------------------------------- fail-safe + +class TestNothingRaises: + + def test_a_git_timeout_degrades(self, tmp_path, monkeypatch): + def _boom(*_a, **_k): + raise subprocess.TimeoutExpired(cmd="git", timeout=1) + monkeypatch.setattr(cs.subprocess, "run", _boom) + + assert cs._git_query(tmp_path, ["status"]) == (None, True) + + def test_an_oserror_from_git_degrades(self, tmp_path, monkeypatch): + def _boom(*_a, **_k): + raise OSError("no exec") + monkeypatch.setattr(cs.subprocess, "run", _boom) + + window = cs.resolve_window(tmp_path) + + assert window.available is False + assert window.reason == cs.REASON_GIT_UNAVAILABLE + + def test_an_existing_but_unreadable_log_is_never_an_available_empty_selection(self, tmp_path): + """The false green: is_file() passes on mode 000, and read_events swallows the + open failure and yields nothing — so this reported "no decisions" having read + none. Regression for the exact defect class this module exists to prevent.""" + if os.name == "nt": + pytest.skip("POSIX permission bits do not apply on Windows") + if os.geteuid() == 0: + pytest.skip("root bypasses file permissions") + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + log.chmod(0o000) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + try: + selection = cs.select_events(window, path=log) + finally: + log.chmod(0o644) + + assert selection.available is False, "unreadable must not present as an empty window" + assert selection.reason == cs.REASON_LOG_UNREADABLE + assert selection.events == () + + def test_hostile_event_shapes_do_not_raise(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = tmp_path / "d.jsonl" + log.write_text( + json.dumps({"ts": 12345, "run_id": None}) + "\n" + + json.dumps({"ts": ["list"], "run_id": {"d": 1}}) + "\n" + + json.dumps("a bare string") + "\n", + encoding="utf-8", + ) + + selection = cs.select_events(window, path=log) + + assert selection.available is True + assert selection.events == () + + +# ---------------------------------------------------------------------- invariants + +class TestInvariants: + + @pytest.mark.parametrize("reason_name", [ + n for n in dir(cs) if n.startswith("REASON_") and n != "REASON_OK" + ]) + def test_every_reason_is_human_readable_prose(self, reason_name): + value = getattr(cs, reason_name) + assert isinstance(value, str) and value + assert value == value.lower() or value[0].islower(), "reason reads as prose, not a code" + + def test_an_unavailable_result_always_carries_a_reason(self, tmp_path): + for result in (cs.resolve_window(tmp_path), cs.select_events(cs.ChangeWindow())): + assert result.available is False + assert result.reason, "unavailable without a reason is the silent failure" + + def test_an_available_window_carries_no_reason(self, tmp_path): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + + assert cs.resolve_window(repo).reason == cs.REASON_OK + + +class TestPrivacy: + + def test_no_reason_string_leaks_a_path_home_or_username(self): + import os + home = os.path.expanduser("~") + user = os.environ.get("USER") or os.environ.get("USERNAME") or "" + for name in dir(cs): + if not name.startswith("REASON_"): + continue + value = getattr(cs, name) + assert os.sep not in value, f"{name} contains a path separator" + assert home not in value + if user: + assert user not in value + + def test_this_process_opens_no_socket(self, tmp_path, monkeypatch): + """Scope stated honestly: patching `socket` covers **this** process only. It + says nothing about what the `git` child does, so the companion test below + checks the git side by a different means.""" + def _no_sockets(*_a, **_k): + raise AssertionError("change_summary must not open a socket") + monkeypatch.setattr(socket, "socket", _no_sockets) + + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + window = cs.resolve_window(repo) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + + assert window.available is True + assert cs.select_events(window, path=log).available is True + + def test_no_git_subcommand_can_reach_a_remote(self, tmp_path, monkeypatch): + """The git side of the no-network claim, which the socket patch above cannot + observe: a child process has its own sockets. Rather than pretending to watch + them, this asserts the only thing that actually matters — every subcommand + issued is a local read, so none of them has a remote to reach.""" + remote_capable = { + "fetch", "pull", "push", "clone", "remote", "ls-remote", + "submodule", "archive", "bundle", "daemon", "send-pack", "fetch-pack", + } + issued: list = [] + + def _capture(args, **_kwargs): + issued.append(list(args)) + raise OSError("not run") + + # The repo is built first: `subprocess` is shared, so patching it before the + # fixture would intercept the fixture's own git calls. + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + window = cs.ChangeWindow( + project_root=str(repo), base_sha=_git(repo, "rev-parse", "HEAD"), + since="2026-01-01T00:00:00+00:00", available=True, + ) + + monkeypatch.setattr(cs.subprocess, "run", _capture) + cs.resolve_window(repo) + # The linkage half lives in a later change; exercise it when present so this + # test covers every git call site on whichever branch it runs. + linker = getattr(cs, "link_changed_files", None) + if linker is not None: + linker(repo, window) + + assert issued, "the helper must actually have been exercised" + for argv in issued: + subcommand = argv[1] if len(argv) > 1 else "" + assert subcommand not in remote_capable, f"remote-capable: {subcommand}" + + +class TestDeterminism: + + def test_the_same_state_yields_identical_results(self, tmp_path): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + _commit(repo, "b.txt") + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T00:00:00+00:00", "a"), _event("2026-06-02T00:00:00+00:00", "b"), + ]) + + windows, selections = [], [] + for _ in range(5): + window = cs.resolve_window(repo) + windows.append(window) + selections.append(cs.select_events(window, path=log)) + + assert len({(w.base_ref, w.base_sha, w.since) for w in windows}) == 1 + assert len({(tuple(s.runs), s.scanned, s.undated) for s in selections}) == 1 + + +class TestTheLogIsBoundToTheWindowsProject: + """A window and its events must describe the same project. + + `resolve_window` takes an explicit root; `decision_log.default_log_path()` defaults + to the cwd. Those are independent inputs, so a window for project A resolved while + the process sat in project B used to select B's decisions — one project's changes + reported alongside another's history. + """ + + def test_the_default_log_follows_the_window_not_the_cwd(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + window = cs.resolve_window(repo) + monkeypatch.delenv("CFS_DECISION_LOG", raising=False) + seen = {} + + def _record(start=None): + seen["start"] = start + return tmp_path / "log.jsonl" + + monkeypatch.setattr(decision_log, "default_log_path", _record) + cs._default_log_for(window) + + assert seen["start"] == Path(window.project_root), "the window's root, not the cwd" + + def test_the_window_records_the_project_it_describes(self, tmp_path): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + + assert cs.resolve_window(repo).project_root == str(repo) + + def test_even_an_unavailable_window_records_its_project(self, tmp_path): + """Provenance must not depend on success, or a failure reason loses its subject.""" + assert cs.resolve_window(tmp_path).project_root == str(tmp_path) + + def test_a_rootless_window_still_falls_back_to_the_cwd_default(self, monkeypatch): + monkeypatch.delenv("CFS_DECISION_LOG", raising=False) + monkeypatch.setattr(decision_log, "default_log_path", lambda start=None: None) + + assert cs._default_log_for(cs.ChangeWindow(available=True)) == (None, False) + + def test_the_real_resolver_finds_the_windows_project_from_another_cwd( + self, tmp_path, monkeypatch, + ): + """The earlier test patched `default_log_path`, so it proved the root was + *passed* — not that passing it works. This drives the real resolver against two + genuine Studio projects with the process standing in the wrong one.""" + monkeypatch.delenv("CFS_DECISION_LOG", raising=False) + wanted = _make_studio_project(tmp_path / "wanted") + other = _make_studio_project(tmp_path / "other") + monkeypatch.chdir(other) + + resolved, _overridden = cs._default_log_for( + cs.ChangeWindow(project_root=str(wanted), available=True), + ) + + assert resolved is not None + assert wanted.resolve() in resolved.parents, "the window's project, not the cwd's" + assert other.resolve() not in resolved.parents + + def test_a_root_that_is_not_a_studio_project_is_reported(self, tmp_path, monkeypatch): + """The non-empty-root branch: a root was recorded, but it is not a project.""" + monkeypatch.delenv("CFS_DECISION_LOG", raising=False) + window = cs.ChangeWindow( + project_root=str(tmp_path), since="2026-01-01T00:00:00+00:00", available=True, + ) + + selection = cs.select_events(window) + + assert selection.available is False + assert selection.reason == cs.REASON_NOT_A_PROJECT + + +class TestTheAnswerDoesNotDependOnWhereTheProcessStands: + """The window carries its project so the log cannot come from the cwd. That only + holds if the recorded root is absolute — a relative one reintroduces the exact + dependence, since both `default_log_path` and `subprocess(cwd=...)` resolve at use + time rather than at capture time.""" + + def test_a_relative_root_is_stored_resolved(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.chdir(repo) + + window = cs.resolve_window(Path(".")) + + assert Path(window.project_root).is_absolute() + assert Path(window.project_root) == repo.resolve() + + def test_a_window_survives_a_later_chdir(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.chdir(repo) + window = cs.resolve_window(Path(".")) + + monkeypatch.chdir(tmp_path) # the process moves after capture + + assert Path(window.project_root) == repo.resolve(), "the recorded root must not move" + + def test_an_ambient_git_dir_cannot_redirect_the_query(self, tmp_path, monkeypatch): + """`cwd=` alone is not enough: GIT_DIR overrides it, so a query about one + project could be answered from another's repository.""" + here = _make_repo(tmp_path / "here") + elsewhere = _make_repo(tmp_path / "elsewhere") + # The two repositories must be genuinely distinguishable. `_make_repo` writes + # identical content with a fixed identity, so two repos created in the same + # second produce the *same* commit sha — which made the first version of this + # test unfalsifiable: it compared a value against its own twin and passed + # whether or not the redirect had been neutralised. A distinct commit in the + # decoy is what gives the assertion something to fail on. + _commit(elsewhere, "decoy.txt", "different content\n") + + # Both shas are captured *before* the redirect is installed. Reading the + # expected value afterwards would route the test's own helper through the very + # mechanism under test, comparing wrong against wrong. + expected = _git(here, "rev-parse", "HEAD") + decoy = _git(elsewhere, "rev-parse", "HEAD") + assert expected != decoy, "fixture precondition: the repositories must differ" + _point_ref(here, "refs/remotes/upstream/main", expected) + monkeypatch.setenv("GIT_DIR", str(elsewhere / ".git")) + monkeypatch.setenv("GIT_WORK_TREE", str(elsewhere)) + + window = cs.resolve_window(here) + + assert window.available is True + assert window.base_sha == expected, "answered from `here`" + assert window.base_sha != decoy, "and not from the redirect target" + + def test_the_sanitised_environment_drops_every_redirect_variable(self, monkeypatch): + for name in cs._GIT_REDIRECT_VARS: + monkeypatch.setenv(name, "/somewhere/else") + + env = cs._git_env() + + assert not [n for n in cs._GIT_REDIRECT_VARS if n in env] + assert "PATH" in env, "the rest of the environment is preserved" + + +class TestCallerValuesCannotBecomeGitOptions: + + def test_a_ref_beginning_with_a_dash_is_treated_as_a_ref(self, tmp_path): + """Without `--end-of-options` git reads this as an option, so the failure is + an argument error rather than an honest "no such ref".""" + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + + window = cs.resolve_window(repo, base="--upload-pack=touch /tmp/pwned") + + assert window.available is False + assert window.reason == cs.REASON_BASE_REF_UNKNOWN, "refused as a ref, not as an option" + + def test_a_ref_containing_nul_is_refused_not_raised(self, tmp_path): + """`subprocess` raises ValueError for an embedded NUL before git starts — + outside the git helper's handler, so this escaped the never-raises contract.""" + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + + window = cs.resolve_window(repo, base="main\x00") + + assert window.available is False + assert window.reason == cs.REASON_BASE_REF_UNKNOWN, "a ref that cannot exist" + + def test_every_option_bearing_call_separates_its_operands(self): + """A structural check, so a new call site that interpolates a caller value + without the separator is caught here rather than in review.""" + import inspect + source = inspect.getsource(cs) + required = ['"--verify", "--quiet", "--end-of-options"', + '["merge-base", "--end-of-options"', + '"--format=%cI", "--end-of-options"'] + if "_git_records" in source: + # Only present once the linkage half lands. + required.append('"--name-status", "-z", "--end-of-options"') + for fragment in required: + assert fragment in source, f"missing separator: {fragment}" + + +class TestGitFailureIsNotAConclusionAboutHistory: + """After the repo probe succeeds, a later git failure must not be reported as a + finding about the branch. "No merge base" and "git timed out" are different facts.""" + + def test_a_tool_failure_during_merge_base_reports_git_not_history(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.setattr(cs, "_merge_base", lambda *_a, **_k: (None, True)) + + window = cs.resolve_window(repo) + + assert window.reason == cs.REASON_GIT_UNAVAILABLE + assert window.reason != cs.REASON_NO_MERGE_BASE + + def test_a_genuine_absence_of_a_merge_base_still_says_so(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.setattr(cs, "_merge_base", lambda *_a, **_k: (None, False)) + + assert cs.resolve_window(repo).reason == cs.REASON_NO_MERGE_BASE + + def test_a_tool_failure_reading_the_base_time_reports_git(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + _point_ref(repo, "refs/remotes/upstream/main", _git(repo, "rev-parse", "HEAD")) + monkeypatch.setattr(cs, "_commit_time", lambda *_a, **_k: (None, True)) + window = cs.resolve_window(repo) + + assert window.reason == cs.REASON_GIT_UNAVAILABLE + assert window.base_sha, "what was already learned is still reported" + + def test_a_non_zero_exit_is_a_valid_negative_not_a_tool_failure(self, tmp_path): + """`merge-base` and `rev-parse --verify` exit non-zero to mean "no" — treating + that as breakage would mislead in the opposite direction.""" + repo = _make_repo(tmp_path / "r") + + value, failed = cs._git_query(repo, ["rev-parse", "--verify", "--quiet", "nope"]) + + assert value is None + assert failed is False + + def test_git_not_launching_is_a_tool_failure(self, tmp_path, monkeypatch): + def _boom(*_a, **_k): + raise OSError("no git") + monkeypatch.setattr(cs.subprocess, "run", _boom) + + assert cs._git_query(tmp_path, ["status"]) == (None, True) + + +class TestAnExplicitSinceIsValidatedUpFront: + """A caller-supplied bound is the caller's input, so a complaint about it must name + that input. Accepting it and failing later produced a reason about a base commit + that was never consulted.""" + + @pytest.mark.parametrize("bad", [ + "nonsense", "", "2026-13-99T00:00:00+00:00", "not-a-time", + ]) + def test_an_unparseable_bound_never_yields_an_available_window(self, bad, tmp_path): + window = cs.resolve_window(tmp_path, since=bad) + + assert window.available is False + if bad: + assert window.reason == cs.REASON_INVALID_SINCE + + def test_a_naive_bound_is_refused_rather_than_assumed_utc(self, tmp_path): + """Guessing an offset would silently shift the window boundary.""" + window = cs.resolve_window(tmp_path, since="2026-06-01T00:00:00") + + assert window.available is False + assert window.reason == cs.REASON_INVALID_SINCE + + def test_a_valid_bound_still_short_circuits_git(self): + window = cs.resolve_window(Path("/nonexistent-abc"), since="2026-01-01T00:00:00+00:00") + + assert window.available is True + assert window.reason == cs.REASON_OK + + +class TestBaseRefLookupPreservesTheGitDiagnosis: + """The previous round fixed this for merge-base and commit-time but left base-ref + lookup on the value-only helper, so two of three stages were covered.""" + + def test_a_tool_failure_on_an_explicit_ref_reports_git(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + monkeypatch.setattr(cs, "_git_query", lambda *_a, **_k: (None, True)) + monkeypatch.setattr(cs, "_detect_repo", lambda *_a, **_k: cs.REASON_OK) + + window = cs.resolve_window(repo, base="release") + + assert window.reason == cs.REASON_GIT_UNAVAILABLE + assert window.reason != cs.REASON_BASE_REF_UNKNOWN + + def test_a_tool_failure_on_the_default_ref_reports_git(self, tmp_path, monkeypatch): + repo = _make_repo(tmp_path / "r") + monkeypatch.setattr(cs, "_git_query", lambda *_a, **_k: (None, True)) + monkeypatch.setattr(cs, "_detect_repo", lambda *_a, **_k: cs.REASON_OK) + + window = cs.resolve_window(repo) + + assert window.reason == cs.REASON_GIT_UNAVAILABLE + assert window.reason != cs.REASON_NO_BASE_REF + + def test_a_genuinely_missing_explicit_ref_still_says_so(self, tmp_path): + repo = _make_repo(tmp_path / "r") + + assert cs.resolve_window(repo, base="no-such").reason == cs.REASON_BASE_REF_UNKNOWN + + def test_the_default_walk_stops_at_the_first_tool_failure(self, tmp_path, monkeypatch): + """Walking the remaining candidates after a launch failure would report a fact + about the repository that was never established.""" + repo = _make_repo(tmp_path / "r") + calls = [] + + def _fail(_root, args): + calls.append(args) + return None, True + + monkeypatch.setattr(cs, "_git_query", _fail) + monkeypatch.setattr(cs, "_detect_repo", lambda *_a, **_k: cs.REASON_OK) + cs.resolve_window(repo) + + assert len(calls) == 1, "one attempt, not one per candidate ref" + + +class TestTheLogIsReadExactlyOnce: + """Readability, the events and the line count come from one read of the file. + + Separate reads let two things go wrong. A valid line appended between the event + read and the line count made the count exceed the events, so ordinary concurrent + logging was reported as corruption. A rotation between the readability probe and + the event read swapped the verified file for a fresh near-empty one, and the + selection came back clean and almost empty with no sign anything had moved. + """ + + def test_the_log_is_opened_exactly_once(self, tmp_path, monkeypatch): + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + real_open = Path.open + opens = [] + + def _counting(self, *args, **kwargs): + if self == log: + opens.append(args) + return real_open(self, *args, **kwargs) + + monkeypatch.setattr(Path, "open", _counting) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=log) + + assert selection.available is True + assert len(opens) == 1, "a second open is a gap for the file to change in" + + def test_a_line_appended_during_selection_is_not_reported_as_corruption( + self, tmp_path, monkeypatch, + ): + """With separate reads the line count saw the new line and the events did not, + so `skipped_lines` reported normal concurrent activity as corruption.""" + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + real_parse = decision_log.parse_events + + def _append_then_parse(lines): + with log.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(_event("2026-06-01T00:00:01+00:00", "late")) + "\n") + return real_parse(lines) + + monkeypatch.setattr(decision_log, "parse_events", _append_then_parse) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=log) + + assert selection.skipped_lines == 0 + assert selection.scanned == 1, "the snapshot, not the file as it is now" + + def test_a_rotation_during_selection_cannot_swap_the_file_under_the_read( + self, tmp_path, monkeypatch, + ): + """The writer rotates by renaming the log aside and starting a fresh one. A + selection that verified the old file and then read the new one reported a + clean, near-empty window; now what was verified is what is read.""" + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + real_parse = decision_log.parse_events + + def _rotate_then_parse(lines): + os.replace(log, log.with_name(log.name + ".1")) + log.write_text("", encoding="utf-8") + return real_parse(lines) + + monkeypatch.setattr(decision_log, "parse_events", _rotate_then_parse) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=log) + + assert len(selection.events) == 1, "the file that was read, not its replacement" + assert selection.skipped_lines == 0 + + def test_a_read_error_is_unreadable_not_absent(self, tmp_path, monkeypatch): + """The plain `OSError` arm — neither a missing file nor a permission bit — so a + regression confined to it fails here rather than hiding behind the arms other + tests drive. Line coverage marks a whole `except (A, B)` covered once either + fires, which is how this arm went untested before.""" + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + + def _io_error(self, *_a, **_k): + raise OSError(5, "Input/output error") + + monkeypatch.setattr(Path, "read_text", _io_error) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=log) + + assert selection.available is False + assert selection.reason == cs.REASON_LOG_UNREADABLE, "a failed read is not proof of absence" + + def test_a_log_path_under_a_file_is_absent_not_unreadable(self, tmp_path): + """`NotADirectoryError`: a parent component is a regular file, so no log can + exist there. That is absence — the writer's own mkdir fails the same way.""" + (tmp_path / "cache").write_text("", encoding="utf-8") + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=tmp_path / "cache" / "d.jsonl") + + assert selection.reason == cs.REASON_LOG_ABSENT + + def test_an_empty_log_is_a_count_not_a_failure(self, tmp_path): + empty = tmp_path / "empty.jsonl" + empty.write_text("", encoding="utf-8") + + assert cs._read_log(empty) == ([], cs.REASON_OK) + assert cs._read_log(tmp_path / "gone.jsonl") == (None, cs.REASON_LOG_ABSENT) + + def test_skipped_lines_is_exact_for_the_snapshot(self, tmp_path): + """Two non-empty lines that yield no event — one not JSON, one JSON that is not + an object — and a blank line, which is not a line at all for this purpose.""" + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log( + tmp_path / "d.jsonl", + [_event("2026-06-01T00:00:00+00:00")], + extra="{ this is not json\n\n[]\n", + ) + + selection = cs.select_events(window, path=log) + + assert selection.skipped_lines == 2 + + +class TestRunIdsAreCanonicalised: + + @pytest.mark.parametrize("raw,expected", [ + ("abcdef012345", "abcdef012345"), + ("ABCDEF012345", "abcdef012345"), # case variants are one run, not two + (" ab12 ", "ab12"), # surrounding whitespace is not identity + (" ", ""), # whitespace is not a run + ("", ""), + (None, ""), + (1, ""), # a non-string must not join its own text + ("not-hex!", "not-hex!"), # unrecognised, but a real identifier + ("Custom-Run-7", "custom-run-7"), # a future writer's shape is not rejected + ("STRASSE", "strasse"), + ("Straße", "strasse"), # casefold, not lower: ß folds to ss + ]) + def test_the_canonical_form(self, raw, expected): + assert cs._canonical_run_id(raw) == expected + + def test_unicode_case_variants_form_one_group(self, tmp_path): + """`lower()` left these as two runs while the docstring promised casefolding.""" + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T00:00:00+00:00", "Straße"), + _event("2026-06-01T00:00:01+00:00", "STRASSE"), + ]) + + selection = cs.select_events(window, path=log) + + assert selection.runs == ("strasse",), "one logical run, not two" + + def test_case_variants_form_one_group(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T00:00:00+00:00", "ABCDEF012345"), + _event("2026-06-01T00:00:01+00:00", "abcdef012345"), + ]) + + selection = cs.select_events(window, path=log) + + assert selection.runs == ("abcdef012345",), "one logical run, not two" + assert len(cs.group_by_run(selection)["abcdef012345"]) == 2 + + def test_a_numeric_id_does_not_merge_with_its_own_text(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = tmp_path / "d.jsonl" + log.write_text( + json.dumps({"schema": 1, "ts": "2026-06-01T00:00:00+00:00", "run_id": 1}) + "\n" + + json.dumps({"schema": 1, "ts": "2026-06-01T00:00:01+00:00", "run_id": "1"}) + "\n", + encoding="utf-8", + ) + + grouped = cs.group_by_run(cs.select_events(window, path=log)) + + assert set(grouped) == {cs.RUN_UNATTRIBUTED, "1"} + assert len(grouped[cs.RUN_UNATTRIBUTED]) == 1 + assert len(grouped["1"]) == 1 + + def test_a_whitespace_id_is_unattributed_not_a_named_run(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00", " ")]) + + selection = cs.select_events(window, path=log) + + assert selection.runless == 1 + assert selection.runs == (cs.RUN_UNATTRIBUTED,) + + +class TestUndecodableLogs: + + def test_invalid_utf8_returns_unavailable_rather_than_raising(self, tmp_path): + """The probe opened the file but did not decode it, so `read_events` performed + the first strict read and UnicodeDecodeError escaped `select_events`.""" + log = tmp_path / "bad.jsonl" + log.write_bytes(b'{"schema":1,"ts":"2026-06-01T00:00:00+00:00"}\n\xff\xfe bad\n') + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=log) + + assert selection.available is False + assert selection.reason == cs.REASON_LOG_UNREADABLE + + +class TestEveryEventIsGrouped: + + def test_events_without_a_run_id_are_bucketed_and_counted(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = tmp_path / "d.jsonl" + rows = [ + _event("2026-06-01T00:00:00+00:00", "r1"), + _event("2026-06-01T00:00:01+00:00", ""), + {"schema": 1, "ts": "2026-06-01T00:00:02+00:00", "event": "x", "payload": {}}, + {"schema": 1, "ts": "2026-06-01T00:00:03+00:00", "run_id": None, + "event": "x", "payload": {}}, + ] + log.write_text("".join(json.dumps(r) + "\n" for r in rows), encoding="utf-8") + + selection = cs.select_events(window, path=log) + grouped = cs.group_by_run(selection) + + assert selection.runless == 3 + assert sum(len(v) for v in grouped.values()) == len(selection.events), \ + "every selected event lands in exactly one bucket" + assert grouped[cs.RUN_UNATTRIBUTED] and len(grouped[cs.RUN_UNATTRIBUTED]) == 3 + + def test_a_collision_with_the_unattributed_label_merges_rather_than_loses(self): + """This assertion used to rest on run ids being hexadecimal. That is no longer + enforced — an unrecognised id is kept rather than discarded — so a writer + emitting this exact parenthesised string would land in the same bucket. The + property worth pinning is therefore the *consequence*: such events merge into + one bucket, and none is dropped. Losing an event would be the real defect; + sharing a label with an anonymous one is cosmetic.""" + selection = cs.EventSelection( + events=({"run_id": cs.RUN_UNATTRIBUTED}, {"run_id": None}), + runs=(cs.RUN_UNATTRIBUTED,), + available=True, + ) + + grouped = cs.group_by_run(selection) + + assert sum(len(v) for v in grouped.values()) == 2 + + +class TestGrouping: + + def test_grouping_preserves_run_order(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [ + _event("2026-06-01T00:00:01+00:00", "z"), + _event("2026-06-01T00:00:02+00:00", "a"), + _event("2026-06-01T00:00:03+00:00", "z"), + ]) + selection = cs.select_events(window, path=log) + + grouped = cs.group_by_run(selection) + + assert list(grouped) == ["z", "a"], "run order follows the log, not the alphabet" + assert [len(v) for v in grouped.values()] == [2, 1] + + def test_grouping_an_empty_selection_is_empty_not_an_error(self): + assert cs.group_by_run(cs.EventSelection()) == {} + + +class TestRepoDetectionKeepsItsAnswersApart: + """`rev-parse --is-inside-work-tree` has three outcomes — true, false, and a + non-zero exit — plus the tool not answering at all. Each is a different fact.""" + + def test_a_tool_failure_during_detection_is_not_a_verdict_about_the_directory( + self, tmp_path, monkeypatch, + ): + """Before: the flag was dropped and `git --version` was asked separately, so a + timeout on the real question plus a healthy second launch came back as "not a + git repository" — a claim nothing had established.""" + repo = _make_repo(tmp_path / "r") + real_run = subprocess.run + + def _flaky(cmd, *args, **kwargs): + if "--is-inside-work-tree" in cmd: + raise subprocess.TimeoutExpired(cmd=cmd, timeout=1) + return real_run(cmd, *args, **kwargs) + + monkeypatch.setattr(cs.subprocess, "run", _flaky) + + window = cs.resolve_window(repo) + + assert window.available is False + assert window.reason == cs.REASON_GIT_UNAVAILABLE, "nothing learned, nothing claimed" + + def test_a_bare_repository_is_a_repository_without_a_working_tree(self, tmp_path): + bare = tmp_path / "bare.git" + bare.mkdir() + _git(bare, "init", "-q", "--bare") + + window = cs.resolve_window(bare) + + assert window.available is False + assert window.reason == cs.REASON_NO_WORK_TREE, "a bare repository is a repository" + + def test_the_git_directory_itself_is_outside_the_working_tree(self, tmp_path): + repo = _make_repo(tmp_path / "r") + + assert cs.resolve_window(repo / ".git").reason == cs.REASON_NO_WORK_TREE + + def test_detection_launches_git_exactly_once(self, tmp_path, monkeypatch): + """The second launch was the `--version` guess. It is gone, and with it the + doubled cost on every non-repo path.""" + real_run = subprocess.run + launches = [] + + def _counting(cmd, *args, **kwargs): + launches.append(cmd) + return real_run(cmd, *args, **kwargs) + + monkeypatch.setattr(cs.subprocess, "run", _counting) + + assert cs.resolve_window(tmp_path).reason == cs.REASON_NOT_A_REPO + assert len(launches) == 1 + + +class TestResultsAreImmutableRecords: + """A selection's counts describe its collections. If a caller could grow, shrink or + reassign them, `scanned`, `undated` and `runless` would silently stop being true.""" + + def test_a_window_refuses_reassignment(self, tmp_path): + window = cs.resolve_window(tmp_path) + + with pytest.raises(dataclasses.FrozenInstanceError): + window.since = "2026-01-01T00:00:00+00:00" + + def test_a_selection_refuses_reassignment_and_its_collections_cannot_grow(self, tmp_path): + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + selection = cs.select_events(window, path=log) + + with pytest.raises(dataclasses.FrozenInstanceError): + selection.scanned = 0 + assert isinstance(selection.events, tuple) + assert isinstance(selection.runs, tuple) + + def test_grouping_shares_the_selections_event_objects_deliberately(self, tmp_path): + """Not a copy: one event, one identity. A copy would let a renderer annotate a + group and then read a different value back from `events`.""" + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00", "r1")]) + selection = cs.select_events(window, path=log) + + grouped = cs.group_by_run(selection) + + assert grouped["r1"][0] is selection.events[0] + + +class TestAnEnvironmentOverrideIsFollowedAndReported: + """`$CFS_DECISION_LOG` is process-wide: the writer honours it in every project the + process runs in. The reader follows it — that is where the events are — and says + so, because one shared log cannot be attributed to the window's project.""" + + def test_the_override_is_read_because_that_is_where_the_writer_wrote( + self, tmp_path, monkeypatch, + ): + """Reading the project-local path instead would report "no decision log yet" + about a log that exists and is being written to.""" + project = _make_studio_project(tmp_path / "project") + shared = _write_log(tmp_path / "shared.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + monkeypatch.setenv("CFS_DECISION_LOG", str(shared)) + window = cs.ChangeWindow( + project_root=str(project), since="2026-01-01T00:00:00+00:00", available=True, + ) + + selection = cs.select_events(window) + + assert selection.available is True + assert len(selection.events) == 1 + assert selection.log_overridden is True, "a shared log is reported as shared" + + def test_the_writer_and_the_reader_agree_on_where_the_log_is(self, tmp_path, monkeypatch): + """End to end: the writer records from inside project B with the override set, + and a window for project A reads that event. Had the reader ignored the + override, it would have found nothing at all.""" + project_a = _make_studio_project(tmp_path / "a") + project_b = _make_studio_project(tmp_path / "b") + shared = tmp_path / "shared.jsonl" + monkeypatch.setenv("CFS_DECISION_LOG", str(shared)) + monkeypatch.setattr(decision_log, "opt_out_sentinel_path", lambda: tmp_path / "absent") + monkeypatch.setattr(decision_log, "_FAILURE_WARNED", False) + monkeypatch.chdir(project_b) + assert decision_log.record("validation", {"check": "x"}, command="validate") is True + window = cs.ChangeWindow( + project_root=str(project_a), since="2000-01-01T00:00:00+00:00", available=True, + ) + + selection = cs.select_events(window) + + assert selection.scanned == 1 + assert selection.log_overridden is True + + def test_a_project_resolved_log_is_not_reported_as_overridden(self, tmp_path, monkeypatch): + monkeypatch.delenv("CFS_DECISION_LOG", raising=False) + project = _make_studio_project(tmp_path / "project") + log = decision_log.default_log_path(project) + assert log is not None + log.parent.mkdir(parents=True, exist_ok=True) + _write_log(log, [_event("2026-06-01T00:00:00+00:00")]) + window = cs.ChangeWindow( + project_root=str(project), since="2026-01-01T00:00:00+00:00", available=True, + ) + + selection = cs.select_events(window) + + assert len(selection.events) == 1 + assert selection.log_overridden is False + + def test_an_explicit_path_is_never_reported_as_overridden(self, tmp_path, monkeypatch): + """A caller that names the log chose it; the environment did not.""" + monkeypatch.setenv("CFS_DECISION_LOG", str(tmp_path / "elsewhere.jsonl")) + log = _write_log(tmp_path / "d.jsonl", [_event("2026-06-01T00:00:00+00:00")]) + window = cs.ChangeWindow(since="2026-01-01T00:00:00+00:00", available=True) + + selection = cs.select_events(window, path=log) + + assert len(selection.events) == 1, "the named log, not the override" + assert selection.log_overridden is False + + +class TestTheBoundaryFollowsTheMergeBase: + """The lower bound is the merge-base's commit time, so it moves when the merge-base + does. This pins that documented behaviour — and its remedy — so a change to the + window's meaning is a deliberate edit here rather than a silent drift.""" + + def test_a_rebase_advances_the_boundary_and_an_explicit_since_restores_it( + self, tmp_path, monkeypatch, + ): + def _at(stamp): + monkeypatch.setenv("GIT_AUTHOR_DATE", stamp) + monkeypatch.setenv("GIT_COMMITTER_DATE", stamp) + + _at("2026-01-01T00:00:00+00:00") + repo = _make_repo(tmp_path / "r") + trunk = _git(repo, "branch", "--show-current") + old_base = _git(repo, "rev-parse", "HEAD") + _git(repo, "checkout", "-q", "-b", "feature") + _at("2026-01-02T00:00:00+00:00") + _commit(repo, "feature.txt") + _git(repo, "checkout", "-q", trunk) + _at("2026-01-05T00:00:00+00:00") + new_base = _commit(repo, "upstream.txt") + _point_ref(repo, "refs/remotes/upstream/main", new_base) + _git(repo, "checkout", "-q", "feature") + log = _write_log(tmp_path / "d.jsonl", [_event("2026-01-03T00:00:00+00:00")]) + + before = cs.resolve_window(repo) + assert before.base_sha == old_base + assert len(cs.select_events(before, path=log).events) == 1 + + _git(repo, "rebase", "-q", "upstream/main") + + after = cs.resolve_window(repo) + assert after.base_sha == new_base, "the merge-base moved, so the boundary moved" + assert after.since > before.since + assert cs.select_events(after, path=log).events == (), \ + "the documented loss: a decision logged before the new base commit" + + pinned = cs.resolve_window(repo, since=before.since) + assert len(cs.select_events(pinned, path=log).events) == 1, "since= is the remedy" diff --git a/vulture_whitelist.py b/vulture_whitelist.py index dbf3375f..61ca10bd 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -17,6 +17,14 @@ from studio.utils.eval_judge import Gold from studio.utils.manifest import ManifestLayerState from studio.utils.okf import write_concept_file +from studio.utils.change_summary import ( + resolve_window, + select_events, + group_by_run, + ChangeWindow, + EventSelection, + RUN_UNATTRIBUTED, +) is_json = _UI.is_json # staticmethod alias exposed on the ui singleton @@ -131,3 +139,24 @@ SemanticCalibration.excluded # noqa: B018 SemanticCalibration.judge # noqa: B018 SemanticCalibration.schema_version # noqa: B018 + +# Change-summary core: the window, event-selection and linkage API the +# change-summary command will consume. Landed ahead of its CLI wrapper so the +# pure logic is reviewable on its own, so nothing in production calls it yet. +# +# Listed here are the module's entry points plus the result fields that only an +# external consumer reads. Fields the module reads itself are deliberately absent: +# an entry for one of those is a false positive that suppresses a real dead-code +# signal, so if vulture stops flagging a name here it should be removed rather +# than kept "just in case". +resolve_window # noqa: B018 +select_events # noqa: B018 +group_by_run # noqa: B018 +ChangeWindow.base_ref # noqa: B018 +ChangeWindow.base_sha # noqa: B018 +EventSelection.scanned # noqa: B018 +EventSelection.undated # noqa: B018 +EventSelection.skipped_lines # noqa: B018 +EventSelection.runless # noqa: B018 +EventSelection.log_overridden # noqa: B018 +RUN_UNATTRIBUTED # noqa: B018