cleveragents-core/tools/_pr_comments_cache.py

"""Persistent on-disk cache for PR/issue comments with delta fetch.

What this is
------------

Live test 2026-05-13 showed every implementer cycle on PR #30
re-fetched 1340+ accumulated comments via the
``/repos/{owner}/{repo}/issues/{N}/comments`` endpoint, hit the
20-page pagination cap, truncated the most recent comments off the
end, and burned ~30s on the fetch alone. Across 5 attempts in a
single PR cycle that's ~150s of identical work plus stale data.

This module caches the comment list to disk and supports a delta
fetch via Forgejo's ``?since=<ISO timestamp>`` query parameter. The
common case becomes: 5-10 NEW comments since the last fetch (or
zero) rather than the full 1340.

Cache layout
------------

``/tmp/cleveragents-comment-cache/pr-{N}.json`` — per-PR JSON file.
Schema::

    {
      "schema_version": 1,
      "pr_number": 30,
      "fetched_at": "2026-05-13T10:00:00+00:00",
      "since_cursor": "2026-05-13T09:59:00+00:00",
      "comments": [
        {"id": 1, "created_at": "...", "body": "...", ...},
        ...
      ],
      "any_partial_fetch": false
    }

``fetched_at`` is the wall-clock time of the last write — it drives
the staleness check only. ``since_cursor`` is the ``created_at`` of
the newest cached comment; it is what we pass as ``?since=`` on the
next delta fetch, so the delta backfills forward from where the
cache actually ends. A page-capped seed holds only the OLDEST N
comments, so its newest cached comment is NOT the newest on the PR —
using a wall-clock ``?since=`` would skip the un-fetched middle
forever. The list is append-only by id. ``any_partial_fetch`` is
True when the cache was seeded from a page-capped fetch and has not
yet been backfilled to completeness by a later delta.

Invalidation
------------

Comments edited or deleted upstream are NOT detected in v1. Two
defenses:

- ``CACHE_MAX_STALENESS_S`` (default 24h) — when the cache is older
  than this, we discard it and do a full refetch. Bounds the
  staleness window.
- Force-refresh via ``invalidate(pr_number)`` for operator-side
  manual interventions.

Edits and deletions are rare in this codebase's PR workflow; the
v1 trade-off (skip detection, do periodic refresh) is intentional.

Concurrency
-----------

The cache file is written atomically (write-to-tmp + rename). Two
dispatchers reading the SAME PR's comments concurrently each
compute the same delta and the last writer wins — idempotent. A
modest amount of duplicate-fetch work is the cost; the alternative
(file locking) would serialise readers, which is worse.

The cache is shared between the reviewer and implementer
dispatchers because both fetch the same endpoint with the same
auth and both benefit equally from the cache.

Errors
------

Every code path is best-effort. Cache read failures fall through
to a full fetch. Delta fetch failures fall through to a full
fetch. Full fetch failures bubble up to the caller as ``(items,
completed=False)`` — same contract as ``_review_fetch._api_get_paginated``.
"""
from __future__ import annotations

import datetime as _dt
import json
import logging
import os
import sys
from pathlib import Path
from typing import Any

_TOOLS_DIR = str(Path(__file__).resolve().parent)
if _TOOLS_DIR not in sys.path:
    sys.path.insert(0, _TOOLS_DIR)
from _loader import (  # noqa: E402  type: ignore[import-not-found]
    load_sibling as _load_sibling,
)

_review_fetch = _load_sibling("_review_fetch", "_review_fetch.py")

_logger = logging.getLogger("pr_comments_cache")

SCHEMA_VERSION = 1

# Default cache directory. Override via env var for tests / archive
# sharing between dispatcher hosts.
_DEFAULT_CACHE_DIR = Path("/tmp/cleveragents-comment-cache")
_CACHE_DIR_ENV = "IMPLEMENTER_DISPATCHER_COMMENT_CACHE_DIR"

# 24 hours — when the cache is older than this we discard it and
# do a full refetch. Bounds the edit/delete staleness window
# without paying the full-fetch cost every cycle.
_DEFAULT_MAX_STALENESS_S = 86400
_STALENESS_ENV = "IMPLEMENTER_DISPATCHER_COMMENT_CACHE_MAX_STALENESS_S"

# Kill switch for tests + operators who want to bypass the cache
# entirely (e.g. while diagnosing a divergence between cached and
# upstream state).
_DISABLE_ENV = "IMPLEMENTER_DISPATCHER_COMMENT_CACHE_DISABLE"


def cache_dir() -> Path:
    """Resolve the on-disk cache directory."""
    return Path(os.environ.get(_CACHE_DIR_ENV) or str(_DEFAULT_CACHE_DIR))


def cache_path(pr_number: int) -> Path:
    """Per-PR cache file path."""
    return cache_dir() / f"pr-{int(pr_number)}.json"


def _max_staleness_s() -> int:
    raw = os.environ.get(_STALENESS_ENV)
    if raw:
        try:
            return max(0, int(raw))
        except ValueError:
            pass
    return _DEFAULT_MAX_STALENESS_S


def is_disabled() -> bool:
    raw = os.environ.get(_DISABLE_ENV, "").strip().lower()
    return raw in {"1", "true", "yes", "on"}


def _now() -> str:
    return _dt.datetime.now(_dt.timezone.utc).isoformat()


def _read_cache(pr_number: int) -> dict[str, Any] | None:
    """Load the cache file for ``pr_number``. Returns ``None`` on any
    error (missing, malformed, schema-mismatch, stale)."""
    target = cache_path(pr_number)
    if not target.exists():
        return None
    try:
        text = target.read_text(encoding="utf-8")
        payload = json.loads(text)
    except (OSError, ValueError) as exc:
        _logger.warning(
            "comment cache read failed for PR #%s at %s: %s",
            pr_number, target, exc,
        )
        return None
    if not isinstance(payload, dict):
        return None
    if payload.get("schema_version") != SCHEMA_VERSION:
        return None
    # Staleness check
    try:
        fetched_at = _dt.datetime.fromisoformat(
            payload.get("fetched_at", "")
        )
        age_s = (
            _dt.datetime.now(_dt.timezone.utc) - fetched_at
        ).total_seconds()
        if age_s > _max_staleness_s():
            _logger.info(
                "comment cache for PR #%s is %.0fs stale (limit %ds); "
                "discarding for full refetch",
                pr_number, age_s, _max_staleness_s(),
            )
            return None
    except (ValueError, TypeError):
        # Bad timestamp → treat as no-cache
        return None
    if not isinstance(payload.get("comments"), list):
        return None
    return payload


def _write_cache(pr_number: int, payload: dict[str, Any]) -> None:
    """Atomic write of the cache file."""
    target = cache_path(pr_number)
    tmp = target.with_suffix(target.suffix + ".tmp")
    try:
        target.parent.mkdir(parents=True, exist_ok=True)
        tmp.write_text(
            json.dumps(payload, indent=2, default=str),
            encoding="utf-8",
        )
        tmp.replace(target)
    except (OSError, TypeError, ValueError) as exc:
        _logger.warning(
            "comment cache write failed for PR #%s at %s: %s",
            pr_number, target, exc,
        )
        try:
            tmp.unlink(missing_ok=True)
        except OSError:
            pass


def _merge_comments(
    cached: list[dict[str, Any]], delta: list[dict[str, Any]],
) -> list[dict[str, Any]]:
    """Append ``delta`` to ``cached``, deduplicated by comment ``id``.

    Forgejo's ``since=`` query is timestamp-based; if a comment was
    created at the boundary timestamp, the server MAY return it in
    both the cached snapshot and the delta. Dedup by id is the
    simplest defence.

    Preserves the order of ``cached`` (oldest first) and appends
    delta items (also oldest-first within delta) at the tail.
    """
    seen_ids: set[Any] = set()
    out: list[dict[str, Any]] = []
    for c in cached:
        cid = c.get("id")
        if cid is not None and cid in seen_ids:
            continue
        if cid is not None:
            seen_ids.add(cid)
        out.append(c)
    for c in delta:
        cid = c.get("id")
        if cid is not None and cid in seen_ids:
            continue
        if cid is not None:
            seen_ids.add(cid)
        out.append(c)
    return out


def _newest_cursor(comments: list[dict[str, Any]]) -> str:
    """Return the ``created_at`` of the newest (last) comment, for use
    as the ``?since=`` delta cursor on the next fetch.

    The comment list is oldest-first, so the last entry is the newest.
    Returns "" when the list is empty or the newest entry has no
    ``created_at`` — the caller falls back to ``fetched_at``.

    Why this and not ``fetched_at`` (wall-clock): a page-capped seed
    holds only the OLDEST N comments, so the newest *cached* comment
    is not the newest on the PR. ``?since=<newest cached created_at>``
    backfills forward from where the cache actually ends;
    ``?since=<wall-clock>`` would skip the un-fetched middle forever.
    """
    if not comments:
        return ""
    last = comments[-1]
    if isinstance(last, dict):
        return str(last.get("created_at") or "")
    return ""


def get_pr_comments(
    cfg: Any, pr_number: int,
    *,
    owner: str | None = None,
    repo: str | None = None,
) -> tuple[list[dict[str, Any]], bool]:
    """Get all comments for the PR, using the on-disk cache for the
    bulk and fetching only the delta.

    Returns ``(comments, completed)`` — same contract as
    :func:`_review_fetch.fetch_pr_comments`. ``completed`` is True
    when both the cache load AND the delta fetch succeeded
    cleanly; False if either was partial.

    ``owner`` / ``repo`` override the values normally read off ``cfg``.
    The shared comment fetch stack only uses ``cfg.owner`` / ``cfg.repo``
    to build the request path; the HTTP transport layer
    (``_claim_runtime.get``) needs just ``token`` / ``request_timeout_s``
    / ``api_retries``. Callers whose ``cfg`` is the narrower
    ``RuntimeContext`` (e.g. ``_claim_runtime``'s sweep, which uses
    module-level ``REPO_OWNER`` / ``REPO_NAME`` constants) pass owner /
    repo explicitly so they can share the cache without growing their
    cfg shape (spec #6).

    When the cache is disabled, missing, or stale, falls back to a
    full fetch (same behaviour as the legacy code path) and seeds
    the cache for next time.

    Seed-on-truncation (R8-5): a full fetch that hits the pagination
    ceiling (``truncated=True``) still seeds the cache — the clipped
    data is valid, just incomplete — marked ``any_partial_fetch=True``.
    The next run's delta fetch (``?since=<since_cursor>``) backfills
    forward from the newest cached comment. Before this fix, a heavy
    PR whose full fetch always truncated NEVER seeded the cache and
    re-paginated from page 1 every single cycle. A genuinely-failed
    fetch (network / 5xx / malformed, ``truncated=False``) still does
    NOT seed — its data is unreliable.
    """
    use_owner = owner or getattr(cfg, "owner", None)
    use_repo = repo or getattr(cfg, "repo", None)
    base_path = (
        f"/repos/{use_owner}/{use_repo}/issues/{int(pr_number)}/comments"
    )
    if is_disabled():
        # Path-based fetch (no cache layer). Uses the same paginator the
        # cache layer uses, so cfg with only RuntimeContext attrs works.
        items, ok = _review_fetch._api_get_paginated(cfg, base_path)
        return items, ok
    cached = _read_cache(pr_number)
    if cached is None:
        # Cache miss → full fetch. Seed on a clean walk OR a page-cap
        # truncation; skip seeding only on a genuine transient failure
        # (its data is unreliable). ``since_cursor`` is the newest
        # comment we actually have, so a truncated seed's next delta
        # backfills forward instead of from a wall-clock ``?since=``.
        items, completed, truncated = _review_fetch._api_get_paginated(
            cfg, base_path, return_truncation=True,
        )
        if completed or truncated:
            _write_cache(pr_number, {
                "schema_version": SCHEMA_VERSION,
                "pr_number": int(pr_number),
                "fetched_at": _now(),
                "since_cursor": _newest_cursor(items),
                "comments": items,
                "any_partial_fetch": not completed,
            })
        return items, completed

    # Cache hit → delta fetch forward from the newest cached comment.
    # Fall back to ``fetched_at`` for caches written before the
    # ``since_cursor`` field existed.
    since = cached.get("since_cursor") or cached.get("fetched_at", "")
    path = f"{base_path}?since={since}" if since else base_path
    delta_items, delta_ok = _review_fetch._api_get_paginated(cfg, path)

    if not delta_ok:
        # Delta fetch was partial. Return the cached bulk + whatever
        # delta we got, but flag ``completed=False`` so the caller
        # marks the data as not-complete. Do NOT overwrite the cache
        # — a partial overwrite would discard cached entries.
        merged = _merge_comments(cached["comments"], delta_items)
        return merged, False

    # Delta succeeded — merge, persist, return. A clean delta walks to
    # the PR's end, so even a previously-truncated seed is now whole:
    # ``any_partial_fetch`` clears.
    merged = _merge_comments(cached["comments"], delta_items)
    cached_payload = {
        "schema_version": SCHEMA_VERSION,
        "pr_number": int(pr_number),
        "fetched_at": _now(),
        "since_cursor": _newest_cursor(merged),
        "comments": merged,
        "any_partial_fetch": False,
    }
    _write_cache(pr_number, cached_payload)
    return merged, True


def invalidate(pr_number: int) -> None:
    """Force-remove the cache entry for a PR.

    Used by:
    - The dispatcher's post-session cleanup, optionally on hook
      failures or other "this PR's state moved underneath us" cases.
    - Operator manual interventions via a future
      ``tools/clear_comment_cache.py`` script.

    Idempotent — missing entry is a no-op.
    """
    target = cache_path(pr_number)
    try:
        target.unlink(missing_ok=True)
    except OSError as exc:
        _logger.warning(
            "comment cache invalidate failed for PR #%s at %s: %s",
            pr_number, target, exc,
        )


__all__ = (
    "SCHEMA_VERSION",
    "cache_dir",
    "cache_path",
    "get_pr_comments",
    "invalidate",
    "is_disabled",
)