Skip to content

docs(claude): weekly CLAUDE.md refresh 2026-09-28 - #30

Merged
kkroo merged 1 commit into
mainfrom
staff-engineer/docs-claude-weekly-refresh-20260928
Sep 29, 2026
Merged

kkroo merged 1 commit into
mainfrom
staff-engineer/docs-claude-weekly-refresh-20260928

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Sep 28, 2026

Copy link
Copy Markdown

Weekly CLAUDE.md maintenance for Blockcast/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.

PR merged title
#29 2026-09-22 fix(recall): bound observation backfill and torch pools
#28 2026-09-23 docs(claude): weekly CLAUDE.md refresh 2026-09-22 (last week's run)

Added

Pruned

  • Nothing. No entry was shipped, closed, or contradicted by this week's merges.

Fixed

  • Nothing. Every path and symbol citation in the file was mechanically re-resolved against head — all resolve. Spot-verified live: _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), the check-unused-code CI job (.github/workflows/test.yml:5302).

Uncertain — needs human review

  1. 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 in engine/cross_encoder.py already 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.

  2. Separate finding — not fixed here, out of scope. fix(recall): bound observation backfill and torch pools #29 added HINDSIGHT_API_RERANKER_LOCAL_TORCH_THREADS but did not document it in hindsight-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.example is inconsistently populated for this family, so I make no claim about step 6. Fixing it would take this diff outside CLAUDE.md; flagging instead.

  3. Nothing deleted, nothing not understood. No senior-engineer system-prompt block and no Architectural-Principles / Anti-Patterns section was touched.

  4. 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.ts carries 11 distinct harness: ids and PLUGIN_ENTRYPOINTS in registry.ts has exactly 7, union = the 18 listed. Recording it so next week's run doesn't re-litigate it.

  5. Branch name deviates from the runbook, necessarily. hindsight has a remote branch literally named docs, so any docs/* ref is rejected (directory file conflict). Used the repo's established staff-engineer/ prefix.

🤖 Generated with Claude Code

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>

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: c3f7a1b9d2e4 contains no schema change at all. It is a pure bookkeeping revision whose _pg_upgrade is 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_THREADS is undocumented in hindsight-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 a CLAUDE.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

  1. No blocking changes requested.
  2. 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.

@kkroo
kkroo merged commit b753e64 into main Sep 29, 2026
99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant