Address FIX items from PR #1 code review #2

Merged

amoussa1229 merged 1 commit from fix/code-review-followups into main

2026-05-15 17:04:18 +00:00

amoussa1229 commented

2026-05-15 17:01:35 +00:00

(Migrated from github.com)

Summary

Resolves the four FIX-level items surfaced in the post-merge review of PR #1. QUESTION item (test coverage scope) left for now.

Gemini retry coverage (models.py): added google.api_core.exceptions.{ResourceExhausted, InternalServerError, ServiceUnavailable, DeadlineExceeded, GatewayTimeout} to the retriable set. Scanner-route runs no longer crash on transient Google failures. RETRIABLE_EXCEPTIONS count: 6 → 11.
Batched embedding (retriever.py): get_or_build_embeddings now collects all cache misses into a list and issues a single embed_documents() call. Cold rebuild of 87 memories dropped from ~30s to ~10s (one HTTPS round-trip vs. 87).
UTC log filenames (telemetry.py, scripts/weekly_summary.py): both modules use datetime.now(UTC).date() for the filename, matching the UTC timestamp field inside each record. Removes the off-by-one near local midnight and aligns with Sea Haven's UTC-for-logs convention.
Failed-run spend visible (scripts/weekly_summary.py): total_cost no longer filters on success, so failed runs that consumed tokens show up in the digest. Added a (incl. $X on failed runs) sub-line for breakdown.

Test plan

ruff check . clean
ruff format --check . clean (12 files)
python3 -c "import models; print(len(models.RETRIABLE_EXCEPTIONS))" returns 11
Forced cold cache rebuild (rm -rf .cache && python3 -c "...") — 87 embeddings in 10.6s
Cache hit on second invocation — 1.5s wall, retrieves project_seahaven_slack_bot, feedback_new_repo_checklist, project_afterhours_shift_manager for a Slack-deploy task
python3 scripts/weekly_summary.py prints with the new (incl. $X on failed runs) line

Deferred

Test coverage QUESTION: review asked whether the golden-set should exercise retrieval-augmented routing too. Not changed here — current tests are a router-only regression net by design.
NIT items: not addressed (DRY of format_memories_for_prompt, atomic cache write, narrower exception catches). Can pick up in a future cleanup if needed.

Rollback

git revert 5378bac — restores previous behavior on all four items.

## Summary Resolves the four FIX-level items surfaced in the post-merge review of PR #1. QUESTION item (test coverage scope) left for now. - **Gemini retry coverage** (`models.py`): added `google.api_core.exceptions.{ResourceExhausted, InternalServerError, ServiceUnavailable, DeadlineExceeded, GatewayTimeout}` to the retriable set. Scanner-route runs no longer crash on transient Google failures. `RETRIABLE_EXCEPTIONS` count: 6 → 11. - **Batched embedding** (`retriever.py`): `get_or_build_embeddings` now collects all cache misses into a list and issues a single `embed_documents()` call. Cold rebuild of 87 memories dropped from ~30s to ~10s (one HTTPS round-trip vs. 87). - **UTC log filenames** (`telemetry.py`, `scripts/weekly_summary.py`): both modules use `datetime.now(UTC).date()` for the filename, matching the UTC `timestamp` field inside each record. Removes the off-by-one near local midnight and aligns with Sea Haven's UTC-for-logs convention. - **Failed-run spend visible** (`scripts/weekly_summary.py`): `total_cost` no longer filters on `success`, so failed runs that consumed tokens show up in the digest. Added a `(incl. $X on failed runs)` sub-line for breakdown. ## Test plan - [x] `ruff check .` clean - [x] `ruff format --check .` clean (12 files) - [x] `python3 -c "import models; print(len(models.RETRIABLE_EXCEPTIONS))"` returns 11 - [x] Forced cold cache rebuild (`rm -rf .cache && python3 -c "..."`) — 87 embeddings in 10.6s - [x] Cache hit on second invocation — 1.5s wall, retrieves `project_seahaven_slack_bot, feedback_new_repo_checklist, project_afterhours_shift_manager` for a Slack-deploy task - [x] `python3 scripts/weekly_summary.py` prints with the new `(incl. $X on failed runs)` line ## Deferred - **Test coverage QUESTION**: review asked whether the golden-set should exercise retrieval-augmented routing too. Not changed here — current tests are a router-only regression net by design. - **NIT items**: not addressed (DRY of `format_memories_for_prompt`, atomic cache write, narrower exception catches). Can pick up in a future cleanup if needed. ## Rollback `git revert 5378bac` — restores previous behavior on all four items.

This repo is archived. You cannot comment on pull requests.

No reviewers