docs(claude): weekly CLAUDE.md refresh 2026-09-28 - #30
Conversation
Record the convention #29 established: bulk data repairs belong in a bounded, resumable hindsight-admin command, not in an Alembic migration that runs on API startup. Co-Authored-By: Paperclip <noreply@paperclip.ing>
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex (applied directly; nested CLI not launched under the k8s Job runtime).
Reviewed head: a04a464
Docs-only: 11 added lines, one file, no executable surface. For a CLAUDE.md change the load-bearing question is whether its factual claims resolve at head, so the review is weighted there rather than at the code lenses, which have no target.
Every claim in the new text was mechanically re-resolved at this head and all resolve:
| claim | verified at head |
|---|---|
backfill-observation-search-vector command exists |
hindsight-api-slim/hindsight_api/admin/cli.py:803 (@app.command(name=...)) |
commits FOR UPDATE SKIP LOCKED batches |
admin/cli.py:744, rationale at :709 |
| migration is "now-inert" | ..._backfill_observation_search_vector.py:65 — _pg_upgrade is a bare return |
| "a docstring may record the rollout" | that migration's docstring does exactly this |
| "Migrations run automatically on API startup" | corroborated twice — pre-existing CLAUDE.md:160, and #29's own migration docstring (lines 29-30) |
Critical Issues (0)
Important Issues (0)
Suggestions (2)
-
[native-codex]
CLAUDE.md:211— "Keep the migration to the schema change (a docstring may record the rollout)" is imprecise against its own cited exemplar:c3f7a1b9d2e4contains no schema change at all. It is a pure bookkeeping revision whose_pg_upgradeis an empty slot. A reader told to "keep the migration to the schema change" who then opens the pattern-to-copy finds a migration with nothing in it.- The non-obvious convention is in that migration's own docstring and is worth one clause here: the empty slot is deliberate, "rather than making the revision a missing branch, so every schema records that the safe, non-blocking rollout is complete." Suggested: "Keep the migration to the schema change if there is one — when the writer fix lives elsewhere, still land an inert revision whose docstring records the rollout, so every schema registers it."
-
[gstack/review] PR description, "Uncertain #2" — the finding that
HINDSIGHT_API_RERANKER_LOCAL_TORCH_THREADSis undocumented inhindsight-docs/docs/developer/configuration.md, contrary to this file's own Adding New API Configuration Flags step 5, is correct and well-controlled (all four sibling flags are documented, so it is a genuine gap, not an unwritten convention). Correctly scoped out of aCLAUDE.md-only diff — but once this merges it survives only in a PR description. Worth a tracking issue so it is not rediscovered by next week's refresh.
Strengths
- The addition is traceable to a merge, not invented: it documents the exact trap #29 had to undo, and the section previously showed only schema-change examples — so it actively invited the shape #29 reverted. That is the right reason to add a convention.
- Citations are precise and resolve, including the harder "now-inert" claim, which required reading the migration body rather than trusting the filename.
- The PR body's "Pruned: nothing / Fixed: nothing" entries are backed by a stated re-resolution pass rather than left blank, and item 4 records a stale-looking-but-correct harness count specifically so next week does not re-litigate it. That is the expensive half of a docs refresh and it was actually done.
- The declined change is the better call. Leaving the torch-thread convention out because
engine/cross_encoder.py's comment "already sits exactly where someone would need it" is sound, and it is not inconsistent with adding this one: a reader about to write a migration opens this section first and would never think to read an unrelated existing migration's docstring, whereas someone touching the reranker is already in the file. The asymmetry tracks where the reader actually is. - Placement is clean — the paragraph sits between existing prose and a numbered list that already started cold, so no sentence/list pairing is broken and the ordered list still renders as one list.
Recommended Action
- No blocking changes requested.
- Merge once the remaining required CI checks finish green.
Self-review: this PR is authored by app/allyblockcast, so GitHub bars the author from APPROVE. Submitted as a formal COMMENTED review under the Ally App — the artifact of record — per the self-review policy.
Weekly
CLAUDE.mdmaintenance forBlockcast/hindsight(BLO-37564, dispatcher BLO-37496).merged_prs=2 limit=500(gh pr list --state merged --search "merged:>=2026-09-21" --limit 500).N < LIM, so the merge list is complete — no truncation. Both PRs were read in full; nothing was triaged away.Added
c3f7a1b9d2e4observationsearch_vectorrepair from a blocking startupUPDATEinto the resumablehindsight-admin backfill-observation-search-vectorcommand (FOR UPDATE SKIP LOCKED, batched commits). The section previously showed only schema-change examples, so it actively invited the shape fix(recall): bound observation backfill and torch pools #29 had to undo. Cited paths verified present atmainhead2962645.Pruned
Fixed
_fetch_with_per_bank_index_plan(engine/search/retrieval.py:34),fetch_unit_dates(engine/db/ops_postgresql.py:857),/health,/health/ready,/health/live(api/http.py:5343,5360,5377), thecheck-unused-codeCI job (.github/workflows/test.yml:5302).Uncertain — needs human review
Torch-thread convention deliberately NOT added. fix(recall): bound observation backfill and torch pools #29 also capped PyTorch intra/inter-op pools (
DEFAULT_RERANKER_LOCAL_TORCH_THREADS = 2) because PyTorch defaults to host CPU count and "one rerank spawned 20+ workers" in a 6-CPU pod. That is a real invisible-regression trap of the same family as Keeping Postgres Indexes Usable, and it generalises to any local in-process model provider. I left it out because the 8-line comment inengine/cross_encoder.pyalready sits exactly where someone would need it, and duplicating it here earns a second place to go stale. Overrule me if you'd rather it be a documented convention.Separate finding — not fixed here, out of scope. fix(recall): bound observation backfill and torch pools #29 added
HINDSIGHT_API_RERANKER_LOCAL_TORCH_THREADSbut did not document it inhindsight-docs/docs/developer/configuration.md, which this file's own Adding New API Configuration Flags step 5 requires. Control check: all four sibling flags (..._MAX_CONCURRENT,..._FORCE_CPU,..._MODEL,..._FP16) are documented there, so this is a genuine gap rather than an undocumented convention..env.exampleis inconsistently populated for this family, so I make no claim about step 6. Fixing it would take this diff outsideCLAUDE.md; flagging instead.Nothing deleted, nothing not understood. No senior-engineer system-prompt block and no Architectural-Principles / Anti-Patterns section was touched.
Harness registry counts re-verified, left alone. The "11 harnesses / 7 harnesses / 18 ids" claim looked stale under a first grep. It is correct:
hook-lifecycle.tscarries 11 distinctharness:ids andPLUGIN_ENTRYPOINTSinregistry.tshas exactly 7, union = the 18 listed. Recording it so next week's run doesn't re-litigate it.Branch name deviates from the runbook, necessarily.
hindsighthas a remote branch literally nameddocs, so anydocs/*ref is rejected (directory file conflict). Used the repo's establishedstaff-engineer/prefix.🤖 Generated with Claude Code