Close first librarian promote round-trip + add --accept flag

scripts/librarian_promote.py gains a --accept flag that copies the
generated draft to the suggested docs/research/ target in the same
invocation. This is explicit per-invocation opt-in, not auto-apply:
the operator types --accept knowingly, the cp is still a deliberate
acceptance decision, just expressed in one command instead of two.

docs/research/option-a-bench-result.md is the first promoted note,
generated by gemini from
docs/archive/option-a-bench-result-2026-05-07.md and accepted
verbatim. Spot-check verified that cited file:line locations match
current source (traversal.pyx:473-475 and
inference_server.py:226-228 carry the cited code), the Last-verified
header reflects today's date and HEAD, and the durable conclusion
(sync-blocking policy boundary as the structural ceiling, not IPC
plumbing) is preserved.

This closes the end-to-end loop the librarian was designed for:
oversize check surfaced docs/performance.md, the routing pass split
durable analysis into the archive entry, the promote dispatcher
turned the archive entry into a research draft, and the human
accept step landed it as a tracked research note. Took one LLM call.

Removes the now-resolved ignore-list entry for
docs/research/option-a-bench-result.md (the file exists; the
forward reference is real).

scripts/librarian.sh exits 0 against the working tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-08 00:31:59 +09:00
co-authored by Claude Opus 4.7
parent 8bbed31670
commit f5ce49682e
4 changed files with 110 additions and 11 deletions
+18
View File
@@ -193,6 +193,10 @@ shells out to the LLM CLI selected by `LIBRARIAN_LLM`
`runs/tmp/librarian-promote-<timestamp>-draft.md` for human review;
the script never writes into `docs/research/` itself. `--show-prompt`
prints the assembled prompt for inspection without calling the LLM.
`--accept` is an explicit per-invocation opt-in that copies the draft
to the suggested target as part of the same command — still
propose-only by design (the operator chooses acceptance knowingly,
not auto-applied).
Refuses to run if:
- the path is not under `docs/archive/`,
@@ -202,6 +206,20 @@ Refuses to run if:
If the LLM judges the archive non-promotable, it is instructed to
return a single line `SKIP: <reason>` instead of a draft.
### First successful round-trip (2026-05-08)
Smoke test against `docs/archive/option-a-bench-result-2026-05-07.md`
with `LIBRARIAN_LLM=gemini` produced a draft that passed spot-check:
`Last verified` header used today's date and the current commit
(`8bbed31`); cited file paths and line numbers were verified as
real (`traversal.pyx:473-475`, `inference_server.py:226-228`); code
snippets matched current source; durable conclusion (sync-blocking
policy boundary as the structural ceiling) preserved; ~60 lines of
prose, no bullet soup. Accepted verbatim as
`docs/research/option-a-bench-result.md`. The whole loop —
oversize finding → extracted archive → LLM draft → human accept —
closed without touching the LLM's output.
## Stage 2 remaining
- MEMORY.md drift fixup mode (read drift report, propose one-line
+61
View File
@@ -0,0 +1,61 @@
# Option A Benchmark and Structural Ceiling
**Last verified:** 2026-05-08, commit `8bbed31`
**Source:** `docs/archive/option-a-bench-result-2026-05-07.md`
## Question
Why did the centralized inference server (Option A) result in a 5x traversal regression (0.21x speedup) despite the GPU being ~230x faster at raw forward passes than the CPU?
Short answer: **the sync-blocking nature of the current traversal recursion prevents batching.** Because every worker thread waits for a single-row policy response before proceeding, the realized GPU batch size is limited by the worker count, which is too small to amortize the IPC and shared-memory synchronization overhead for small models.
## Code reference
The sync-blocking boundary is located in `src/coolrl_lost_cities/games/classic/deep_cfr/traversal.pyx`. In `_regret_matching_policy_c` (around line 475) and `_policy_from_strategy_network` (around line 552), the traversal recursion crosses into PyTorch:
```cython
with torch.inference_mode():
x = torch.as_tensor(info_state, dtype=torch.float32, device=self.device).unsqueeze(0)
advantages = networks[player](x).squeeze(0).detach().cpu().numpy().astype(np.float32)
```
When `traversal.inference_backend` is set to `server` in `configs/deep_cfr/default.yaml`, the `networks[player]` call is intercepted by a `NetworkProxy` (instantiated in `workers.py`, around line 91). This proxy posts a request to the `InferenceServer` and blocks until a response is received via a per-slot event.
The server's batching logic in `src/coolrl_lost_cities/games/classic/deep_cfr/inference_server.py` (around line 221) reports the realized batch size:
```python
handles.stats_queue.put(BatchStatsMessage(batch_size=len(batch), group_count=len(groups)))
```
## Analysis
The 2026-05-07 benchmark results (`scripts/bench_inference_backend.py`) showed that with `num_workers: 8`, the mean batch size reported by the server was only **~7.27.9**. Because the server further splits batches by network kind and index to avoid mixing weights in a single forward pass, the actual GPU group size was roughly **4 rows**.
Comparing the per-state costs from the microbenchmark (`scripts/profile_gpu_forward.py`):
| Realized Batch | μs/state (GPU) |
| ---: | ---: |
| 1 | 80.07 |
| 4 | 20.30 |
| 64 | 1.46 |
| 256 | 0.34 |
At a batch size of 4, the GPU compute time (~20μs) is negligible compared to the IPC round-trip cost (queue post, context switch, event wakeup), which is on the order of **100200μs** per call. This overhead is compounded over ~200k policy calls per iteration, leading to the observed jump from ~10.8s (local CPU) to ~51.6s (remote GPU) for the traversal phase.
The "structural ceiling" is that `batch_window_us` and `max_batch` tuning cannot improve performance if there are no additional in-flight requests to coalesce. With 8 workers blocking synchronously, the server will never see the 64+ requests needed to reach the high-efficiency SIMD regime.
## Practical implication
Option A is deferred for the current small MLP models (512x3). The `local` backend remains the default in `configs/deep_cfr/default.yaml`.
To unlock the projected GPU gains, the traversal must be restructured to drive batch sizes up. This leads to two primary paths:
1. **Option B (Interleaved Traversal):** Refactor the Cython traversal into a state machine that can advance multiple traversals concurrently per worker. Each worker would suspend at a policy call, batch its own requests, and resume continuations once the results return.
2. **Option C (Vectorized Traversal):** Re-implement traversal as a single-process operation on large tensors.
Option B is the chosen next step because it preserves the Cython game-rule logic while breaking the sync-blocking boundary. Option A's plumbing remains in the codebase as it will become beneficial if (a) the model size increases to the point where forward compute exceeds IPC cost, or (b) Option B successfully drives the server's batch size toward 64+.
## References
- `docs/research/batched-traversal-inference-decision.md`
- `experiments/traversal_policy_boundary/bench_policy_boundary.py`
- `docs/archive/post-a-optimization-calculus-2026-05-07.md`
-5
View File
@@ -18,8 +18,3 @@ scripts/run_model_size_experiment.sh
# Future config described in docs/plans/torch_compile.md
configs/deep_cfr/default_compile.yaml
# Hypothetical accept target named in docs/plans/librarian.md as a
# smoke-test illustration; resolves naturally if Stage 2 promote is
# accepted, otherwise stays a forward reference.
docs/research/option-a-bench-result.md
+27 -2
View File
@@ -24,6 +24,7 @@ from __future__ import annotations
import argparse
import os
import re
import shutil
import subprocess
import sys
from datetime import datetime
@@ -32,7 +33,7 @@ from pathlib import Path
LLM_COMMANDS = {
"claude": ["claude", "-p"],
"codex": ["codex", "exec"],
"gemini": ["gemini", "-p"],
"gemini": ["gemini", "-p", ""],
}
DATE_SUFFIX = re.compile(r"-\d{4}-\d{2}-\d{2}$")
@@ -85,6 +86,16 @@ def main() -> int:
action="store_true",
help="Print the assembled prompt to stdout and exit; do not call the LLM.",
)
parser.add_argument(
"--accept",
action="store_true",
help=(
"After the draft lands in runs/tmp/, also copy it verbatim to the "
"suggested docs/research/ target. Explicit per-invocation opt-in: "
"still propose-only by design (you are choosing acceptance "
"knowingly), not auto-apply."
),
)
args = parser.parse_args()
root = _repo_root()
@@ -145,7 +156,8 @@ def main() -> int:
file=sys.stderr,
)
result = subprocess.run(
cmd + [prompt],
cmd,
input=prompt,
capture_output=True,
text=True,
check=False,
@@ -157,10 +169,23 @@ def main() -> int:
draft_path.write_text(result.stdout, encoding="utf-8")
print(f"Draft written to: {draft_path.relative_to(root)}")
if args.accept:
if target.exists():
print(
f"Refusing --accept: {rel_target} appeared while the LLM ran.",
file=sys.stderr,
)
return 1
shutil.copyfile(draft_path, target)
print(f"Copied to: {rel_target}")
print("Next: review the diff and `git add` + commit if you're satisfied.")
else:
print(f"Suggested target on accept: {rel_target}")
print()
print("Next: review the draft. To accept verbatim:")
print(f" cp {draft_path.relative_to(root)} {rel_target}")
print("Or rerun with --accept to copy in the same step.")
return 0