From 5378bac69b1641e9e751b74aedf4cb9b9a538b91 Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Fri, 15 May 2026 13:01:14 -0400 Subject: [PATCH] Address code review FIX items from PR #1 Follow-up to the Phases 1-2-4 modernization PR. - models.py: extend retry coverage to Gemini transient errors (google.api_core: ResourceExhausted, InternalServerError, ServiceUnavailable, DeadlineExceeded, GatewayTimeout). Before this, a 429 or 5xx from Google would crash the scanner route. Retriable count went from 6 to 11. - retriever.py: batch cache-miss embeddings into a single embed_documents() call instead of one embed_query() per memory. Cold rebuild of 87 memories went from ~30s to ~10s; one HTTPS round-trip instead of 87. - telemetry.py + scripts/weekly_summary.py: use UTC date for log filenames so they align with the UTC `timestamp` field inside each record. Eliminates the off-by-one near local midnight and matches Sea Haven's UTC-for-logs convention. - scripts/weekly_summary.py: include failed runs in the total cost estimate (they consumed tokens too) and surface a `(incl. $X on failed runs)` breakdown so outages are visible in the digest. Validated: ruff clean; weekly_summary.py still prints; retriever cold rebuild + cache hit both verified end-to-end. --- models.py | 14 ++++++++++++++ retriever.py | 21 +++++++++++++-------- scripts/weekly_summary.py | 8 +++++--- telemetry.py | 8 ++++++-- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/models.py b/models.py index 4feb0d4..ef6cfbc 100644 --- a/models.py +++ b/models.py @@ -40,6 +40,20 @@ def _collect_retriable_exceptions() -> tuple[type[BaseException], ...]: ) except ImportError: pass + try: + from google.api_core import exceptions as google_exceptions + + excs.extend( + [ + google_exceptions.ResourceExhausted, + google_exceptions.InternalServerError, + google_exceptions.ServiceUnavailable, + google_exceptions.DeadlineExceeded, + google_exceptions.GatewayTimeout, + ] + ) + except ImportError: + pass return tuple(excs) diff --git a/retriever.py b/retriever.py index d59ccbe..3e4d328 100644 --- a/retriever.py +++ b/retriever.py @@ -78,22 +78,27 @@ def _save_cache(cache: dict) -> None: def get_or_build_embeddings(memories: list[Memory]) -> dict[str, list[float]]: """Return {memory_name: embedding}. Rebuilds entries whose file mtime changed; preserves the rest. Drops cache entries for deleted memories. + + Cache misses are embedded in a single batched `embed_documents` call so a + cold rebuild is one HTTPS round-trip instead of one per memory. """ cache = _load_cache() - embedder: OpenAIEmbeddings | None = None out: dict[str, list[float]] = {} - dirty = False + misses: list[Memory] = [] for m in memories: cached = cache.get(m.name) if cached and cached.get("mtime") == m.mtime: out[m.name] = cached["embedding"] - continue - if embedder is None: - embedder = _embedder() - vec = embedder.embed_query(m.content) - out[m.name] = vec - cache[m.name] = {"mtime": m.mtime, "embedding": vec} + else: + misses.append(m) + + dirty = False + if misses: + vectors = _embedder().embed_documents([m.content for m in misses]) + for m, vec in zip(misses, vectors, strict=True): + out[m.name] = vec + cache[m.name] = {"mtime": m.mtime, "embedding": vec} dirty = True valid_names = {m.name for m in memories} diff --git a/scripts/weekly_summary.py b/scripts/weekly_summary.py index 89b91c7..609f369 100755 --- a/scripts/weekly_summary.py +++ b/scripts/weekly_summary.py @@ -36,7 +36,8 @@ COST_RATES: dict[str | None, tuple[float, float]] = { def load_recent_records(window_days: int = WINDOW_DAYS) -> list[dict]: - today = datetime.date.today() + # UTC date aligns with telemetry.py's log filename convention. + today = datetime.datetime.now(datetime.UTC).date() out: list[dict] = [] for i in range(window_days): day = today - datetime.timedelta(days=i) @@ -78,7 +79,8 @@ def summarize(records: list[dict], window_days: int = WINDOW_DAYS) -> str: (r.get("tokens_in", 0) or 0, r.get("tokens_out", 0) or 0) ) - total_cost = sum(estimate_cost(r) for r in records if r.get("success")) + total_cost = sum(estimate_cost(r) for r in records) + failed_cost = sum(estimate_cost(r) for r in records if not r.get("success")) total_tokens_in = sum((r.get("tokens_in", 0) or 0) for r in records) total_tokens_out = sum((r.get("tokens_out", 0) or 0) for r in records) @@ -104,7 +106,7 @@ def summarize(records: list[dict], window_days: int = WINDOW_DAYS) -> str: "", "## Spend (rough)", f"- Total tokens: {total_tokens_in:,} in / {total_tokens_out:,} out", - f"- Estimated cost: **${total_cost:.2f}**", + f"- Estimated cost: **${total_cost:.2f}** (incl. ${failed_cost:.2f} on failed runs)", "", "## Tokens per route (mean in/out per run)", ] diff --git a/telemetry.py b/telemetry.py index 9408413..01f5943 100644 --- a/telemetry.py +++ b/telemetry.py @@ -79,10 +79,14 @@ def build_record( def log_run(record: dict, log_dir: Path = LOG_DIR) -> None: - """Append a single record as one JSON line. Never raises.""" + """Append a single record as one JSON line. Never raises. + + Filename uses the UTC date so it stays aligned with the UTC `timestamp` + field inside the record (no off-by-one near local midnight). + """ try: log_dir.mkdir(parents=True, exist_ok=True) - date = datetime.date.today().isoformat() + date = datetime.datetime.now(datetime.UTC).date().isoformat() path = log_dir / f"{date}.jsonl" with path.open("a") as f: f.write(json.dumps(record) + "\n")