You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
src/brainlayer/cli/__init__.py — the Typer app root — had no mapping. The generic src/brainlayer/*.py rule derives tests/test___init__.py from the basename, finds nothing, and escalates the whole unit suite. Practical effect: no CLI change on this repo could be pushed scoped. Every one demanded a full-suite run.
Not theoretical. It blocked a denylist PR whose only unmapped path was this file, on a machine already running two other full suites — where test_arbitration.py::test_real_concurrent_writers_keep_interactive_store_searchable_under_sla hung at 0.0% CPU for 12 minutes under the load, and an earlier attempt took a SIGTERM at 95% with zero FAILED lines.
The fix
Names the suites that actually drive the app through CliRunner: the test_cli_*.py set, plus test_setup_mcp_migration.py (the setup command's migration path) and test_agent_profiles.py (the agent-profile group).
Fail-closed, per the rule #762 established for src/brainlayer/__init__.py: ALL named suites must exist to count as mapped. A per-file mapped=1 would let a deleted or renamed suite silently narrow this gate while it still looked mapped — partial coverage here is worse than none. A missing sibling escalates to the full suite instead, which is the safe side. test_cli_root_mapping_is_fail_closed_when_a_named_suite_is_missing pins that behaviour.
Verification
test_changed_only_scope_maps_cli_root_to_the_cli_command_suitesfails on the base commit (no mapping → falls back), passes with it.
Against the real tests directory: 0 escalations, and the unit invocation carries exactly the 9 mapped files. test_engine.py / test_vector_store.py appear only as the pre-existing --ignore= real-DB exclusions — they are not run.
tests/test_run_tests_script.py: 25 passed.
bash -n clean; shellcheck warning count unchanged from origin/main (7 vs 7) — the SC2221/SC2222 pairs are pre-existing, shifted by the added lines.
Pushed scoped: gate green on all five phases, no full suite.
One honest note: the fail-closed test passes on the base commit too (everything is unmapped there, so it "escalates" for the wrong reason). It is a regression guard for the new mapping, not a red-first test — the first test is the red-first one.
Low Risk
Only the pre-push test scoping script and its contract tests change; no runtime CLI or auth behavior is modified.
Overview Pre-push changed-only scope no longer treats every edit to src/brainlayer/cli/__init__.py as unmapped (which forced a full unit suite). run_tests.sh now has an explicit case that runs 20 pytest files—the suites that import from brainlayer.cli import app—when that path changes.
The mapping is fail-closed like src/brainlayer/__init__.py: all listed suites must exist on disk before any are selected; if one is missing, the path stays unmapped and the script escalates to the full unit suite with a visible unmapped: warning.
test_run_tests_script.py adds a shared CLI_ROOT_SUITES list plus contract tests for the happy path (all suites invoked, no full-tree -v) and for missing-sibling escalation (test_watch_backfill_cli.py omitted).
Reviewed by Cursor Bugbot for commit 2a951ce. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Map src/brainlayer/cli/__init__.py to 20 CLI test suites in run_tests.sh
Updates map_changed_files_to_pytests in run_tests.sh to map src/brainlayer/cli/__init__.py to 20 specific test suites importing the Typer application.
Leaves the source unmapped and falls back to the full unit suite if any of the 20 required test files are missing.
Adds tests in test_run_tests_script.py to verify the successful mapping and fail-closed fallback.
Case ordering. The new case must precede the generic src/brainlayer/*.py case, since * spans / in a bash case glob and would otherwise shadow it. Verified against the real tests dir (0 escalations), but worth a second pair of eyes.
Completeness of the named set. If a CLI command's behaviour is exercised by a suite I did not name, this mapping is a fail-open for that path — that is the failure mode most worth catching here.
Not requesting @codex (standing rule until ~09-07). No Bugbot.
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.
We reviewed changes in 3cc9eca...2a951ce on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Every Value below was measured by this run. A row this machine cannot measure says n/a — <reason> instead of a number; baselines in Notes name their own machine, method and date and were not measured here.
Row
Status
Value (measured by this run)
Method
Notes
commit provenance
🟢 GREEN
measured 2a951cec12cc == PR head · checkout 09afe3dd29c3
commit graph + live PR head · in-process · runner
Which commit this whole table is about. On a pull_request event the checkout is GitHub's synthetic merge ref, whose sha is not on the PR — #759's table printed 13fa724278bf while that PR's head was 4632f979 — so this row names the PR-head parent instead, the sha a reviewer can actually see. The comparison sha is read live from repos/{owner}/{repo}/pulls/{n} when the table is collected, not taken from the event payload, because the payload cannot know the run has been overtaken. Residual window, stated rather than papered over: a push landing between that read and the comment being posted is not caught here — the run for that push refreshes the table.
baseline attestation
🟢 GREEN
baseline f421d1a7c5e6 matches the main attestation (run 33977684550 · main 3cc9eca386bd · 2026-09-05T16:23:10Z)
main attestation artifact via Actions API · in-process · runner
What every comparison is measured AGAINST, and who says so. The baseline fields of tests/fixtures/sprint_gate/corpus.json (queries, latency_baseline_ms, thresholds) are compared to the ratchet-attestation artifact of the latest successful push or (no-input) workflow_dispatch run of ratchet-attest.yml on main, fetched through the Actions API — a PR run cannot write to another run's artifacts. A field that differs is RED unless that main run measured the new value; today no runner-side collector measures any baseline field, so today the baseline cannot move by PR at all, and this row says so instead of a hand edit passing. Boundary: the comparator is this PR's checkout of ci_ratchet_table.py, diff-reviewable, not tamper-proof.
provenance
🟢 GREEN
stamped 09afe3dd29c3 == HEAD, tree clean
wheel stamp · in-process · runner
Sha half of #749 keg-mode provenance: a keg built from this wheel can answer __build_sha__. The helper-age and served-process predicates need a running BrainBar and are measured only by scripts/sprint_gate.py on an installed Mac. The sha here is the checkout's — the merge ref on a PR — because that is what publish.yml stamps at release time; the PR-head sha this table describes is the one in commit provenance above.
mapped bytes
⚪ n/a
n/a — no BrainBar daemon at /tmp/brainbar.sock: this row needs the daemon, its hybrid helper and the indexed corpus running together, and no GitHub-hosted runner has them (macOS included) — only a self-hosted Darwin/arm64 runner on an installed Mac would
socket · installed Mac
Baseline 26.2 GB — installed Mac, socket, 2026-09-03, after R2 drained 15,070 → 0. Up from 16.8 GB because the drain left more vectors mapped under the same cap: the change is the drain, not a leak. Not measured by this run.
search p50/p95
⚪ n/a
n/a — no BrainBar daemon at /tmp/brainbar.sock: this row needs the daemon, its hybrid helper and the indexed corpus running together, and no GitHub-hosted runner has them (macOS included) — only a self-hosted Darwin/arm64 runner on an installed Mac would
socket · installed Mac
Margin p50: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Margin p95: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Calibrated on MacBook-Pro.local at 2026-09-01T08:42:22Z under active_sprint_load (tests/fixtures/sprint_gate/corpus.json). Not measured by this run.
idle CPU
⚪ n/a
n/a — no BrainBar daemon at /tmp/brainbar.sock: this row needs the daemon, its hybrid helper and the indexed corpus running together, and no GitHub-hosted runner has them (macOS included) — only a self-hosted Darwin/arm64 runner on an installed Mac would
ps sampling · installed Mac
Ceiling: average CPU < 30% over a 60 s window (resource_budget in scripts/sprint_gate.py), ratified and kept as a hard budget. Margin daemon: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Margin helper: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Margin watcher: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Needs the BrainBar daemon, helper and watcher actually running. Not measured by this run.
signature_valid
⚪ n/a
n/a — the macOS signature-parity job is trigger-gated and did not run on this PR: it touches no release or signing path (pyproject.toml, scripts/release-*, scripts/brainlayer-version-check.sh, publish.yml, ratchet.yml) and carries no ratchet:signatures label — a GitHub macOS runner bills at ~10× Linux minutes and rebuilds the keg venv from source
codesign · installed keg
scripts/release-verify-signatures.sh <keg> codesign-verifies every *.so/*.dylib under libexec/venv. The macOS parity job installs the published tap formula (etanhey/layers/brainlayer), so this row measures the release path — formula, published sdist and Homebrew's relocation — and not this PR's tree. Release-time baseline for the same keg on a different machine: 442 valid / 0 invalid — installed Mac (M4 Max), brew --prefix brainlayer 1.5.11, 2026-09-03.
🟢 GREEN measured, within budget · 🔴 RED measured, out of budget — a finding to clear before merge · ⚪ n/a not measurable on this machine, never guessed.
No RED rows.
Measured on Linux/x86_64 · measured 2a951cec12cc · PR head 2a951cec12cc · checkout 09afe3dd29c3 · run · updated 2026-09-05 16:34:51 UTC
P1 — Include the omitted root-app CLI suites in the mapping.src/brainlayer/cli/__init__.py registers commands that existing tests exercise through CliRunner, but the mapping does not run those tests. Examples include tests/test_doctor.py (doctor), tests/test_status_truthfulness.py (status), tests/test_watch_backfill_cli.py (watch-backfill), tests/test_runtime_store.py (migrate, migrate-store, and repair-fts), and tests/test_sandbox_db.py (sandbox). A root-app change can therefore be scoped while skipping behavior tests for registered commands. Add all suites that invoke brainlayer.cli.app to cli_root_tests, or use a maintained complete root-app suite set.
The other requested properties are correct:
The script checks all nine named paths before it sets mapped=1. A missing named suite causes escalation.
The src/brainlayer/cli/__init__.py case precedes src/brainlayer/*.py. The generic Bash glob cannot shadow the specific case.
⚠️ Action not completed
Review rate limited.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
…oot hunk verbatim
Measured, not guessed: `changed-only` was falling open to the FULL suite on every push from
this lane, because both changed source paths are unmapped.
WARNING: changed-only scope found an unmapped source change; falling back to full pytest unit suite
WARNING: unmapped: src/brainlayer/index_watchdog.py src/brainlayer/cli/__init__.py
4,553 tests in 7m51s on the M4 -- which LANE-RULES forbids and which hung a sibling seat's
concurrency-SLA test. The comma-separated `BRAINLAYER_CHANGED_FILES` was correct all along; the
mapping table was the real gap, so commas alone never fixed it. After: 0 fallbacks, 133 targeted
tests, whole gate ~22s.
Two mappings, and only one of them is mine.
`src/brainlayer/index_watchdog.py` is mine: the generic `src/brainlayer/X.py -> test_X.py` rule
looks for `test_index_watchdog.py`, which does not exist because the tests are split
unit/boundaryless plus the CLI's own cap suite. Mapped FAIL-CLOSED, per the rule already on main
for `src/brainlayer/__init__.py` (#762): all three must exist to count as mapped, because none
is sufficient alone -- `_unit` drives the watchdog on an injected clock and never starts the
CLI, `_boundaryless` drives the real CLI against stalls that only unwind on interrupt, and
`test_cli_index_watchdog` covers the commit-boundary path the watchdog does not own. A per-file
`mapped=1` would let a renamed suite silently narrow the gate while it still looked mapped.
`src/brainlayer/cli/__init__.py` is NOT mine: PR #783 (`wt/cli-map`, open) already maps it, and
its version is better than the one I first wrote -- mine was per-file `mapped=1`, i.e. the
fail-open that #762 explicitly ruled against. That hunk is carried here BYTE-IDENTICAL to
#783's (verified with diff) purely so this lane can push scoped while #783 is still open; its
tests are #783's too and are not duplicated here. When #783 merges, this branch should rebase
onto main and the duplicate disappears with no conflict, since the content matches exactly.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…river (XS)
The Typer app root had no mapping, so the generic `src/brainlayer/*.py` rule derived
`tests/test___init__.py` from the basename, found nothing, and escalated the whole
unit suite. Practical effect: **no CLI change on this repo could be pushed scoped.**
Same shape #762 fixed for `src/brainlayer/__init__.py`, and not theoretical: it
blocked a denylist PR whose only unmapped path was this file, on a machine already
running two other full suites, where a concurrency-SLA test hangs under the load.
**The named set is DERIVED, not hand-picked** — every tests/ file that imports the
Typer app, re-derivable with:
grep -rl "from brainlayer.cli import app" tests/
That change came out of Cursor's two lows on the first cut, and the sweep they
prompted found the problem was bigger than either:
- `test_setup_mcp_migration.py` was named but never touches the app (it exercises
`brainlayer.setup` directly), so it gated nothing for this path. TAKEN, removed.
- `test_watch_backfill_cli.py` was omitted. TAKEN — and it was 1 of **12** omitted
drivers, not 1 of 1. The hand-picked list named 8 of 20. That is a fail-open, and
it is exactly the risk flagged in the review request.
Fail-closed per the #762 rule: ALL named suites must exist to count as mapped. A
per-file `mapped=1` would let a deleted or renamed suite silently narrow the gate
while it still looked mapped. A missing sibling escalates instead, which is the
safe side. `test_cli_root_mapping_is_fail_closed_when_a_named_suite_is_missing`
pins that.
Verified against the real tests directory: 0 escalations, 20 mapped targets,
`test_setup_mcp_migration.py` absent, `test_watch_backfill_cli.py` present.
`test_engine.py` / `test_vector_store.py` appear only as the pre-existing
`--ignore=` real-DB exclusions.
Rebased onto origin/main (3cc9eca); the conflict was an append-collision with
#775 in tests/test_run_tests_script.py, resolved by keeping both. New subprocess
call site follows the `check=False` + `noqa: S603` style #775 introduced.
Agent: brainlayerClaude-6c94f25f (Opus 5, 1M context)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oot hunk verbatim
Measured, not guessed: `changed-only` was falling open to the FULL suite on every push from
this lane, because both changed source paths are unmapped.
WARNING: changed-only scope found an unmapped source change; falling back to full pytest unit suite
WARNING: unmapped: src/brainlayer/index_watchdog.py src/brainlayer/cli/__init__.py
4,553 tests in 7m51s on the M4 -- which LANE-RULES forbids and which hung a sibling seat's
concurrency-SLA test. The comma-separated `BRAINLAYER_CHANGED_FILES` was correct all along; the
mapping table was the real gap, so commas alone never fixed it. After: 0 fallbacks, 133 targeted
tests, whole gate ~22s.
Two mappings, and only one of them is mine.
`src/brainlayer/index_watchdog.py` is mine: the generic `src/brainlayer/X.py -> test_X.py` rule
looks for `test_index_watchdog.py`, which does not exist because the tests are split
unit/boundaryless plus the CLI's own cap suite. Mapped FAIL-CLOSED, per the rule already on main
for `src/brainlayer/__init__.py` (#762): all three must exist to count as mapped, because none
is sufficient alone -- `_unit` drives the watchdog on an injected clock and never starts the
CLI, `_boundaryless` drives the real CLI against stalls that only unwind on interrupt, and
`test_cli_index_watchdog` covers the commit-boundary path the watchdog does not own. A per-file
`mapped=1` would let a renamed suite silently narrow the gate while it still looked mapped.
`src/brainlayer/cli/__init__.py` is NOT mine: PR #783 (`wt/cli-map`, open) already maps it, and
its version is better than the one I first wrote -- mine was per-file `mapped=1`, i.e. the
fail-open that #762 explicitly ruled against. That hunk is carried here BYTE-IDENTICAL to
tests are #783's too and are not duplicated here. When #783 merges, this branch should rebase
onto main and the duplicate disappears with no conflict, since the content matches exactly.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tually mine
Shell (red since ae3b246, my run_tests.sh commit):
- `tests/*.py|tests/**/*.py` and `src/brainlayer/*.py|src/brainlayer/**/*.py` each raised an
SC2221/SC2222 pair. Both second alternatives are dead: a bash `case` glob's `*` crosses `/`,
verified directly -- `tests/*.py` already matches `tests/x/y/c.py`, and
`src/brainlayer/*.py` already matches `src/brainlayer/cli/__init__.py`. Removed, behaviour
identical, and the nested-path tests still pass. shellcheck on this file: 8 findings on
origin/main -> 4 now. The lines were pre-existing text my mapping commit had only shifted
(241/271 -> 292/322), so DeepSource attributed them to this diff; fixed rather than argued.
Python:
- `Redefining name 'index' from outer scope` -- mine: `_discover_jsonl_files` used `index` as
its enumerate variable, shadowing the module-level `index` command. Renamed to `position`.
- `Call to next() should be wrapped in try-except` x4 -- mine: fishing the cap alarm out with
a bare generator `next()` raises StopIteration instead of saying what went wrong. Replaced
with `_cap_alarm()`, which also asserts there is exactly one.
- `subprocess.run used without explicitly defining check` x3 -- mine: made `check=False`
explicit at my three call sites (these tests assert on returncode themselves). Left the 17
pre-existing call sites alone: identical behaviour, and #783 edits this same file.
- `Body doesn't contain any code` -- mine: a `while ... : pass` busy-wait. Now sleeps 10ms.
Not fixed, with reasons on the PR: `index_fast` cyclomatic complexity (metric only -- measured
18 here against 19 on origin/main, i.e. LOWER than the baseline this PR inherited) and unused
argument `force` (pre-existing on main; `brainlayer index --force` silently does nothing, which
is a real defect and its own PR per canon rule 9).
Also re-synced the carried #783 CLI-root hunk: its seat's rebase replaced the hand-picked list
with a DERIVABLE one after finding the first cut was itself a fail-open (8 drivers named, 12
missed, and one named suite that never touches the app). My copy was the stale version; it is
byte-identical to #783 again.
93 green across the touched suites.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oot hunk verbatim
Measured, not guessed: `changed-only` was falling open to the FULL suite on every push from
this lane, because both changed source paths are unmapped.
WARNING: changed-only scope found an unmapped source change; falling back to full pytest unit suite
WARNING: unmapped: src/brainlayer/index_watchdog.py src/brainlayer/cli/__init__.py
4,553 tests in 7m51s on the M4 -- which LANE-RULES forbids and which hung a sibling seat's
concurrency-SLA test. The comma-separated `BRAINLAYER_CHANGED_FILES` was correct all along; the
mapping table was the real gap, so commas alone never fixed it. After: 0 fallbacks, 133 targeted
tests, whole gate ~22s.
Two mappings, and only one of them is mine.
`src/brainlayer/index_watchdog.py` is mine: the generic `src/brainlayer/X.py -> test_X.py` rule
looks for `test_index_watchdog.py`, which does not exist because the tests are split
unit/boundaryless plus the CLI's own cap suite. Mapped FAIL-CLOSED, per the rule already on main
for `src/brainlayer/__init__.py` (#762): all three must exist to count as mapped, because none
is sufficient alone -- `_unit` drives the watchdog on an injected clock and never starts the
CLI, `_boundaryless` drives the real CLI against stalls that only unwind on interrupt, and
`test_cli_index_watchdog` covers the commit-boundary path the watchdog does not own. A per-file
`mapped=1` would let a renamed suite silently narrow the gate while it still looked mapped.
`src/brainlayer/cli/__init__.py` is NOT mine: PR #783 (`wt/cli-map`, open) already maps it, and
its version is better than the one I first wrote -- mine was per-file `mapped=1`, i.e. the
fail-open that #762 explicitly ruled against. That hunk is carried here BYTE-IDENTICAL to
tests are #783's too and are not duplicated here. When #783 merges, this branch should rebase
onto main and the duplicate disappears with no conflict, since the content matches exactly.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tually mine
Shell (red since ae3b246, my run_tests.sh commit):
- `tests/*.py|tests/**/*.py` and `src/brainlayer/*.py|src/brainlayer/**/*.py` each raised an
SC2221/SC2222 pair. Both second alternatives are dead: a bash `case` glob's `*` crosses `/`,
verified directly -- `tests/*.py` already matches `tests/x/y/c.py`, and
`src/brainlayer/*.py` already matches `src/brainlayer/cli/__init__.py`. Removed, behaviour
identical, and the nested-path tests still pass. shellcheck on this file: 8 findings on
origin/main -> 4 now. The lines were pre-existing text my mapping commit had only shifted
(241/271 -> 292/322), so DeepSource attributed them to this diff; fixed rather than argued.
Python:
- `Redefining name 'index' from outer scope` -- mine: `_discover_jsonl_files` used `index` as
its enumerate variable, shadowing the module-level `index` command. Renamed to `position`.
- `Call to next() should be wrapped in try-except` x4 -- mine: fishing the cap alarm out with
a bare generator `next()` raises StopIteration instead of saying what went wrong. Replaced
with `_cap_alarm()`, which also asserts there is exactly one.
- `subprocess.run used without explicitly defining check` x3 -- mine: made `check=False`
explicit at my three call sites (these tests assert on returncode themselves). Left the 17
pre-existing call sites alone: identical behaviour, and #783 edits this same file.
- `Body doesn't contain any code` -- mine: a `while ... : pass` busy-wait. Now sleeps 10ms.
Not fixed, with reasons on the PR: `index_fast` cyclomatic complexity (metric only -- measured
18 here against 19 on origin/main, i.e. LOWER than the baseline this PR inherited) and unused
argument `force` (pre-existing on main; `brainlayer index --force` silently does nothing, which
is a real defect and its own PR per canon rule 9).
Also re-synced the carried #783 CLI-root hunk: its seat's rebase replaced the hand-picked list
with a DERIVABLE one after finding the first cut was itself a fail-open (8 drivers named, 12
missed, and one named suite that never touches the app). My copy was the stale version; it is
byte-identical to #783 again.
93 green across the touched suites.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ct why the 4h cap missed (#778)
* fix(index): enforce the runtime cap from a watchdog thread, and correct why the 4h cap missed
The M1 ran `com.brainlayer.index` for 12h13m against a 4h cap with no alarm and a stdout log
silent for 12h. The plan blamed the apsw progress handler in `upsert_chunks` for being blind
inside a boundary-less phase. Measured against this apsw (3.51.2.0 / SQLite 3.51.2), that is
not true -- the handler ticks freely, and its `id=`-keyed registration works:
FTS5 rebuild, 400k rows 7,287 ticks / 0.68s
vec0 bulk DELETE 1,008 ticks / 0.042s
vec0 KNN, 1.5M rows 8 ticks / 0.167s (~48/s)
id= registration 34,180 ticks
So a long statement inside `upsert_chunks` would have been stopped. The real hole is
narrower: the cap is enforced ONLY inside `upsert_chunks` and at `index_fast`'s per-file
boundaries. Opening the writer store (schema validation + migrations -- read-heavy, which
matches "100% CPU, WAL unchanged, only DB files open") and the first embedding-model load
have no cap of any kind. Either running long explains 12h with no alarm, with no opcode
blackout needed.
A cap at the work sites cannot cover sites that have none, so this puts it beside them.
`IndexWatchdog` runs its own thread, which a blocked main thread cannot delay:
- alarms `INDEX_RUNTIME_EXCEEDED` the moment the deadline passes, naming the phase, before
the stuck phase unwinds -- unconditional, so it holds even for a phase issuing no SQL;
- calls `sqlite3_interrupt` on the writer connection (apsw documents cross-thread as safe).
Measured: aborts a long FTS5 statement 0.22s into a 0.66s rebuild, leaving
integrity_check ok, the transaction rolled back and every row readable. Retries are
bounded at 3 so the rollback that follows survives;
- leaves `expired` set, which the main thread turns into `IndexDeadlineExceeded`;
- heartbeats phase + last-progress age every `BRAINLAYER_INDEX_HEARTBEAT_S` (default 300s),
so a 12h-silent log cannot recur and the next real stall names its own phase.
Also: file discovery now runs inside the cap window instead of before the clock starts, and
an exception arriving while the watchdog is expired is reported as the cap rather than as an
opaque `Error:`. One cap emits one alarm -- the boundary path adds `phase` and defers to the
watchdog's alarm when it already fired.
The watchdog binds `time.monotonic` at import deliberately: several tests monkeypatch the
CLI's clock, and a watchdog reading a patched clock would both mis-fire and consume values
from those tests' iterators.
Live-checked against an APFS clone of the real 16GB DB, real embedder, never the canonical
path: a 900s-cap run indexed 26 chunks and heartbeated its phase; a 6s cap tripped at 6.153s
naming `embed_and_upsert:<file>` and exited 1; chunks went 805,045 -> 805,062 (committed work
kept), `integrity_check` ok, no dangling transaction.
Not covered: a phase that neither issues SQL nor returns can be alarmed and heartbeaten but
not aborted. Killing such a run is a policy call and its own change.
Tests: the boundary-less cap miss is reproduced first (20.99s wall against a 1.0s cap, zero
interrupts), then closed (3.88s). 27 tests across the two new files plus the 7 pre-existing
cap tests; 110 green over the blast radius.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(index): one deadline on one clock, and make open_store abortable
Round-1 review on #778. Both HIGHs were real; verified in the code, then closed with tests
that fail against the previous commit.
HIGH 1 -- dual deadline. IndexWatchdog computed `own_now + max_runtime_s` at construction, so
arming it after file discovery handed the run a second full budget: a slow rglob plus a stalled
open_store could burn 2 x max_runtime_s, and the PR's claim that discovery is inside the cap
window was false. It now takes `started_at` and the CLI's `monotonic`, so watchdog and CLI
share ONE deadline. Proven: the new test reads 1012.0 against the previous commit where 1007.0
is correct.
HIGH 2 -- open_store, the phase this PR names as the likeliest 12h culprit, could be alarmed
but not aborted, because `interrupt` was attached only after `open_writer_store` returned.
`WriterRuntimeStore._init_runtime_db` creates the apsw connection BEFORE it runs any probe
SQL, so `open_writer_store` now takes an `on_connection` hook and hands the connection over at
that point; the CLI passes `watchdog.set_interrupt`. The whole probe window is interruptible.
Proven: against the previous commit the new test stalls to its 20s ceiling under a 1s cap
("open_store was never interruptible"); with the hook it stops at the cap with
`phase=open_store`. Verified live too -- the hook receives a real apsw.Connection with a
callable `interrupt`, and the store still opens with a valid schema fingerprint.
MEDIUM -- heartbeats stopped at the deadline (`if expired ... elif heartbeat`), so a wedge that
cannot be aborted alarmed once and then went silent again: the second half of the original
12h-silent-log failure. Heartbeats now continue past expiry.
MEDIUM -- `alarm_emitted` was set before the emit succeeded, so a failing emit let the process
exit 1 with no INDEX_RUNTIME_EXCEEDED anywhere, since the CLI defers to that flag.
...and the live-only bug that fixing it introduced: gating solely on "landed" made the alarm
REPEAT. A 6s-cap run against the real DB printed the same alarm 13 times, because
`emit_alarm` returns the Axiom *telemetry* result while its stderr and logging paths are the
documented guaranteed ones -- a falsy return is not a missing record. Now two flags:
`attempted` (set unconditionally, so it can never re-alarm) and `emitted` (only a raising emit
leaves the CLI owing a fallback). Re-checked live: exactly one alarm.
MEDIUM -- tests pinned neither shared-deadline semantics nor an open_store/discovery stall.
Four new tests cover them, each verified to fail against the previous commit.
DeepSource Python (bot round): fixed what is mine -- a redundant `import time` and a `context`
name that shadowed an outer scope. The reimports of `time` and the rich Progress columns, the
`get_db_path` shadow, and unused `force` are pre-existing in `index_fast`; the local reimports
are removed here since they sit in this hunk. **`brainlayer index --force` accepts `--force`
and does nothing with it** -- pre-existing latent bug, its own PR per canon rule 9. On
complexity: `index_fast` measured 19 before this work, 20 after round 1, and **17 now** --
below the baseline I found it at, because one deadline on one clock let three duplicated
`time.monotonic() >= deadline` checks collapse into `watchdog.raise_if_expired()`, which now
checks the clock as well as the flag. Splitting `index_fast` further is a separate refactor.
Live-check (fresh APFS clone of the real 16GB DB, real embedder, BRAINLAYER_DB on the clone
throughout): 900s cap indexed 46 chunks with heartbeats whose `last_progress_age_s` resets per
file; 6s cap tripped at 6.218s naming the phase, exit 1, exactly one alarm, 4 heartbeats;
chunks 805,420 -> 805,466; `integrity_check` ok; no dangling transaction.
Tests: 33 across the two watchdog files, 234 green across the blast radius.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(index): watch discovery too, and stop relabelling real failures as the cap
Round-2 review on 345fb80. Both HIGHs and all three mediums were real; each fix has a guard
proven to fail against 345fb80.
HIGH -- discovery was charged against the cap but not watched. Arming the watchdog after the
rglob fixed only the arithmetic: a walk stuck on a slow mount stayed silent past the deadline
until it returned. The watchdog is now armed BEFORE discovery with `phase=discover`, and
`_discover_jsonl_files` consumes the generator so the cap is checked as the walk proceeds
(every 64 entries -- enough to bound a stall, cheap on a 12,000-file tree). Guard against
345fb80: "the run was silent while discovery was stuck".
HIGH -- after expiry every Exception was rewritten as INDEX_RUNTIME_EXCEEDED. An expired
watchdog means the cap is part of the story, not that this exception IS the cap: a schema
mismatch, an I/O error or an OOM landing during the unwind lost its only actionable cause. The
handler now reports the cap alarm AND keeps the original type and message, on stderr and in
`unwind_error`. Only apsw's `InterruptError` (and `IndexDeadlineExceeded`) are treated as the
interrupt we asked for. Guard against 345fb80: "the original failure was rewritten as the
cap: … cause=RuntimeError" -- the RuntimeError's message was gone.
MEDIUM -- injecting the CLI's clock let the poll thread consume monkeypatched values. Now two
clocks: the caller's, read exactly once by `start()` on the calling thread to carry the
already-spent budget into the wall domain, and this module's import-time wall clock, the only
one the thread reads. One deadline, no shared-clock race. Guard counts caller-clock reads
during a second of polling.
MEDIUM -- `on_connection` failures were swallowed, so an open could look armed while being
non-abortable. Both `runtime_store` paths now log at error level, and the CLI's hook records
`note_uninterruptible(reason)`, which appears in every heartbeat and in the cap alarm.
MEDIUM -- tests did not cover either fail-open hole. Four new tests do, each verified failing
against 345fb80.
LOW -- `raise_if_expired` could trip on the clock without setting `expired` or pulling the
interrupt. It now routes through the same `_on_deadline()` the thread uses, so both halves
agree about the cap.
Also fixed here, and NOT caught by any suite run so far: `tests/test_index_watchdog_boundaryless.py`
poisoned `brainlayer.index_new` for the whole session. `index_new` binds `open_writer_store` at
ITS import time; `_install` patched `runtime_store.open_writer_store` first and imported
`index_new` second, so index_new captured the fake permanently — monkeypatch restores
runtime_store, not index_new — and `test_context_pipeline` then failed with
`'_StallingRuntimeStore' object has no attribute 'upsert_chunks'`. The module is now imported
at test-module scope before any patch, and `_patch_open_writer_store` patches every binding
through monkeypatch. Worth noting: the FULL suite was green on this; the targeted set caught it.
Live-check (fresh APFS clone of the real 16GB DB, real embedder, canonical never touched):
900s cap indexed 38 chunks; 6s cap tripped at 6.203s, exit 1, exactly one alarm, 3 heartbeats;
a 0.5s cap over the real ~12,000-file tree still reached the write phase, so watching discovery
costs nothing measurable. Chunks 805,636 -> 805,674; integrity_check ok; no dangling transaction.
148 green across the blast radius.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(prepush): map index_watchdog fail-closed, and carry #783's CLI-root hunk verbatim
Measured, not guessed: `changed-only` was falling open to the FULL suite on every push from
this lane, because both changed source paths are unmapped.
WARNING: changed-only scope found an unmapped source change; falling back to full pytest unit suite
WARNING: unmapped: src/brainlayer/index_watchdog.py src/brainlayer/cli/__init__.py
4,553 tests in 7m51s on the M4 -- which LANE-RULES forbids and which hung a sibling seat's
concurrency-SLA test. The comma-separated `BRAINLAYER_CHANGED_FILES` was correct all along; the
mapping table was the real gap, so commas alone never fixed it. After: 0 fallbacks, 133 targeted
tests, whole gate ~22s.
Two mappings, and only one of them is mine.
`src/brainlayer/index_watchdog.py` is mine: the generic `src/brainlayer/X.py -> test_X.py` rule
looks for `test_index_watchdog.py`, which does not exist because the tests are split
unit/boundaryless plus the CLI's own cap suite. Mapped FAIL-CLOSED, per the rule already on main
for `src/brainlayer/__init__.py` (#762): all three must exist to count as mapped, because none
is sufficient alone -- `_unit` drives the watchdog on an injected clock and never starts the
CLI, `_boundaryless` drives the real CLI against stalls that only unwind on interrupt, and
`test_cli_index_watchdog` covers the commit-boundary path the watchdog does not own. A per-file
`mapped=1` would let a renamed suite silently narrow the gate while it still looked mapped.
`src/brainlayer/cli/__init__.py` is NOT mine: PR #783 (`wt/cli-map`, open) already maps it, and
its version is better than the one I first wrote -- mine was per-file `mapped=1`, i.e. the
fail-open that #762 explicitly ruled against. That hunk is carried here BYTE-IDENTICAL to
tests are #783's too and are not duplicated here. When #783 merges, this branch should rebase
onto main and the duplicate disappears with no conflict, since the content matches exactly.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(lint): clear the DeepSource Python and Shell findings that are actually mine
Shell (red since ae3b246, my run_tests.sh commit):
- `tests/*.py|tests/**/*.py` and `src/brainlayer/*.py|src/brainlayer/**/*.py` each raised an
SC2221/SC2222 pair. Both second alternatives are dead: a bash `case` glob's `*` crosses `/`,
verified directly -- `tests/*.py` already matches `tests/x/y/c.py`, and
`src/brainlayer/*.py` already matches `src/brainlayer/cli/__init__.py`. Removed, behaviour
identical, and the nested-path tests still pass. shellcheck on this file: 8 findings on
origin/main -> 4 now. The lines were pre-existing text my mapping commit had only shifted
(241/271 -> 292/322), so DeepSource attributed them to this diff; fixed rather than argued.
Python:
- `Redefining name 'index' from outer scope` -- mine: `_discover_jsonl_files` used `index` as
its enumerate variable, shadowing the module-level `index` command. Renamed to `position`.
- `Call to next() should be wrapped in try-except` x4 -- mine: fishing the cap alarm out with
a bare generator `next()` raises StopIteration instead of saying what went wrong. Replaced
with `_cap_alarm()`, which also asserts there is exactly one.
- `subprocess.run used without explicitly defining check` x3 -- mine: made `check=False`
explicit at my three call sites (these tests assert on returncode themselves). Left the 17
pre-existing call sites alone: identical behaviour, and #783 edits this same file.
- `Body doesn't contain any code` -- mine: a `while ... : pass` busy-wait. Now sleeps 10ms.
Not fixed, with reasons on the PR: `index_fast` cyclomatic complexity (metric only -- measured
18 here against 19 on origin/main, i.e. LOWER than the baseline this PR inherited) and unused
argument `force` (pre-existing on main; `brainlayer index --force` silently does nothing, which
is a real defect and its own PR per canon rule 9).
Also re-synced the carried #783 CLI-root hunk: its seat's rebase replaced the hand-picked list
with a DERIVABLE one after finding the first cut was itself a fail-open (8 drivers named, 12
missed, and one named suite that never touches the app). My copy was the stale version; it is
byte-identical to #783 again.
93 green across the touched suites.
Agent: brainlayerClaude-a9949327 (claude-opus-5[1m])
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
All six sites: pyproject.toml, src/brainlayer/__init__.py, server.json
(root + packages[0]), brain-bar/bundle/Info.plist (short, bundle, release).
Casks/brainbar.rb stays 1.5.9: no Swift change this release, declared to
scripts/brainlayer-version-check.sh via
BRAINLAYER_VERSION_CHECK_CASK_LAG_REASON="no BrainBar release for 1.5.16"
(kept in the PR body, never exported into the push env — two tests assert
the check fails without a reason).
Why a release, 22 minutes after the last one. v1.5.15 was tagged at
51a72a0 (20:12:29). #778 — the index runtime watchdog — merged at
78d92bc (20:34:55), so it is NOT in 1.5.15. Verified:
`git merge-base --is-ancestor 78d92bc v1.5.15^{}` -> false, and the
installed keg's python raises
`ModuleNotFoundError: No module named 'brainlayer.index_watchdog'`.
That watchdog is the fix for the M1's nightly `brainlayer index` job,
which ran 14h03m at ~100% CPU on 09-05, 10h past its own 4h cap. Both
Macs' 03:15 index jobs are `launchctl disable`d as a stopgap and come
back ON only after 1.5.16 is installed and the watchdog is proven
present.
Nine commits ride along: #774#777#780#778#783#785#786#787#788.
Co-authored-by: brainlayerClaude-c1601b03 running claude-opus-5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Size: XS. Two files, the #762 shape exactly.
The bug
src/brainlayer/cli/__init__.py— the Typer app root — had no mapping. The genericsrc/brainlayer/*.pyrule derivestests/test___init__.pyfrom the basename, finds nothing, and escalates the whole unit suite. Practical effect: no CLI change on this repo could be pushed scoped. Every one demanded a full-suite run.Not theoretical. It blocked a denylist PR whose only unmapped path was this file, on a machine already running two other full suites — where
test_arbitration.py::test_real_concurrent_writers_keep_interactive_store_searchable_under_slahung at 0.0% CPU for 12 minutes under the load, and an earlier attempt took a SIGTERM at 95% with zeroFAILEDlines.The fix
Names the suites that actually drive the app through
CliRunner: thetest_cli_*.pyset, plustest_setup_mcp_migration.py(thesetupcommand's migration path) andtest_agent_profiles.py(theagent-profilegroup).Fail-closed, per the rule #762 established for
src/brainlayer/__init__.py: ALL named suites must exist to count as mapped. A per-filemapped=1would let a deleted or renamed suite silently narrow this gate while it still looked mapped — partial coverage here is worse than none. A missing sibling escalates to the full suite instead, which is the safe side.test_cli_root_mapping_is_fail_closed_when_a_named_suite_is_missingpins that behaviour.Verification
test_changed_only_scope_maps_cli_root_to_the_cli_command_suitesfails on the base commit (no mapping → falls back), passes with it.test_engine.py/test_vector_store.pyappear only as the pre-existing--ignore=real-DB exclusions — they are not run.tests/test_run_tests_script.py: 25 passed.bash -nclean; shellcheck warning count unchanged fromorigin/main(7 vs 7) — the SC2221/SC2222 pairs are pre-existing, shifted by the added lines.One honest note: the fail-closed test passes on the base commit too (everything is unmapped there, so it "escalates" for the wrong reason). It is a regression guard for the new mapping, not a red-first test — the first test is the red-first one.
🤖 Generated with Claude Code
Note
Low Risk
Only the pre-push test scoping script and its contract tests change; no runtime CLI or auth behavior is modified.
Overview
Pre-push changed-only scope no longer treats every edit to
src/brainlayer/cli/__init__.pyas unmapped (which forced a full unit suite).run_tests.shnow has an explicit case that runs 20 pytest files—the suites that importfrom brainlayer.cli import app—when that path changes.The mapping is fail-closed like
src/brainlayer/__init__.py: all listed suites must exist on disk before any are selected; if one is missing, the path stays unmapped and the script escalates to the full unit suite with a visibleunmapped:warning.test_run_tests_script.pyadds a sharedCLI_ROOT_SUITESlist plus contract tests for the happy path (all suites invoked, no full-tree-v) and for missing-sibling escalation (test_watch_backfill_cli.pyomitted).Reviewed by Cursor Bugbot for commit 2a951ce. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Map
src/brainlayer/cli/__init__.pyto 20 CLI test suites inrun_tests.shmap_changed_files_to_pytestsin run_tests.sh to mapsrc/brainlayer/cli/__init__.pyto 20 specific test suites importing the Typer application.Macroscope summarized 2a951ce.