|
| 1 | +# RSI content agent — retrospective and open design questions |
| 2 | + |
| 3 | +Written after building and running Phase 1 and Phase 2 of `RecursiveSelfImprovingAgentPlan.md` |
| 4 | +against real Talmud pages. That doc is the original design; this one is the evidence gathered |
| 5 | +since, for a fresh, higher-level pass at the architecture. It doesn't try to pre-decide answers — |
| 6 | +where I have a working hypothesis I've said so explicitly, but several of the findings below |
| 7 | +point at real tensions in the original design that are worth reconsidering from scratch rather |
| 8 | +than patching. |
| 9 | + |
| 10 | +PR: [#54](https://github.com/ronshapiro/talmud.page/pull/54), branch |
| 11 | +`worktree-bridge-cse_013nvCXPadZwXNRyPP2mBoCF`, commits `d8e2d119`..`88d135a9`. |
| 12 | + |
| 13 | +## Background (see the original plan for full detail) |
| 14 | + |
| 15 | +Self-hosted, subscription-billed execution: a headless `claude -p` CLI wrapper |
| 16 | +(`rsi_orchestrator/headless_claude.ts`) run on a machine the user controls, not a metered API key |
| 17 | +and not Anthropic's Managed Agents (rejected specifically over separate-billing concerns). The |
| 18 | +generation philosophy was stated as "goal and tools, not a scripted context-assembly function" — |
| 19 | +task-type code points Claude at where relevant data lives in the repo and lets it decide what to |
| 20 | +read, rather than the orchestrator pre-fetching and assembling a curated prompt payload. Derived |
| 21 | +content ships as git-committed JSON, reviewed via PR. A tiered staleness detector |
| 22 | +(`precomputed/rsi_state/staleness.ts`) decides whether an existing artifact still matches its |
| 23 | +source text. Model routing and budget pacing were designed as *learned* over time from logged |
| 24 | +outcomes, not hand-tuned once. |
| 25 | + |
| 26 | +## What's built |
| 27 | + |
| 28 | +**Phase 1** (`precomputed/rsi_state/`, `rsi_orchestrator/`, `js/RsiSuggestionBox.tsx`, |
| 29 | +`express.ts`): the staleness detector (exact-match hash / edit-distance ratio / agentic |
| 30 | +classification hook / structural break), generation-record provenance tracking, a GitHub-issue |
| 31 | +suggestion box in the app UI, and a triage script that reads open suggestions and comments a |
| 32 | +feasibility assessment (no auto-implementation). |
| 33 | + |
| 34 | +**Phase 2** (`rsi_orchestrator/rashi_tosafot_translation.ts` + |
| 35 | +`rashi_tosafot_translation_cli.ts`): the first real content-generation task type. For a book/page, |
| 36 | +finds Rashi/Tosafot comments missing a translation or gone stale, asks Claude to punctuate and |
| 37 | +translate each one, runs a bounded self-critique loop (generate → critique → at most one retry |
| 38 | +with feedback → give up), writes accepted edits to the existing `precomputed/ai_additions/` |
| 39 | +merge mechanism, and records a generation record including model, cost, and now (most recently) |
| 40 | +every tool call Claude made while producing the edit. |
| 41 | + |
| 42 | +## What real runs validated |
| 43 | + |
| 44 | +- **Self-critique catches real errors.** Twice, independently, it rejected a translation for a |
| 45 | + genuine and fairly subtle problem (a wrong number relationship in a measurement passage; an |
| 46 | + invented term not present in the source Hebrew) rather than a superficial one. |
| 47 | +- **Quality holds up on hard content**, not just short comments — a full page-length Tosafot |
| 48 | + passage with nested cross-references to Temurah and Keritot came back coherent and accurate. |
| 49 | +- **The staleness detector and generation-record plumbing work as designed** and are now |
| 50 | + exercised against real data, not just unit tests. |
| 51 | +- **Tool-use instrumentation (added most recently) works** and immediately produced usable |
| 52 | + signal — see below. |
| 53 | + |
| 54 | +## Shortcomings and challenges surfaced by real runs |
| 55 | + |
| 56 | +### 1. Context cost and unproductive "wandering" |
| 57 | + |
| 58 | +The philosophy of pointing Claude at a file path and letting it decide what to read ran into a |
| 59 | +real, measured problem. `cached_outputs/api_request_handler/<Book>.<page>.json` — the only |
| 60 | +context source currently offered beyond the prompt's own inlined source text — is large (345KB |
| 61 | +for one page) and mostly irrelevant to a translation task: a size breakdown of one page showed |
| 62 | +Rashi + Tosafot together are ~100KB of a 476KB file, with **Mesorat Hashas (a citation index, not |
| 63 | +prose) alone accounting for 42% of all commentary bulk**. One real call's usage reported |
| 64 | +`cache_read_input_tokens: 157,342` — on the order of the whole file. |
| 65 | + |
| 66 | +Worse: once tool-use instrumentation existed to actually see what Claude reads (not infer it from |
| 67 | +token counts), the pattern that emerged wasn't "read the whole file for context" — it was |
| 68 | +**searching for worked examples of translation style** across the entire `cached_outputs/` |
| 69 | +directory, including unrelated books (Job, Meilah), plus repeated denied `Bash` attempts, plus at |
| 70 | +one point reading the pipeline's own source code. That single candidate cost **$0.73** — more |
| 71 | +than an entire earlier multi-candidate run. Output quality was fine; the process to get there |
| 72 | +was not. Working hypothesis: the prompt gives Claude the source text directly but no worked |
| 73 | +example of the *expected output*, so it goes hunting for calibration instead of just producing an |
| 74 | +answer. Untested. |
| 75 | + |
| 76 | +This is the crux of the "learned trimming" ask that prompted building the instrumentation: the |
| 77 | +project's stated position was explicitly *not* to hand-pick a commentary allowlist, but rather to |
| 78 | +build toward something that learns the right amount of context per task type from real usage data. |
| 79 | +The instrumentation (`precomputed/rsi_state/context_usage_log.ts`, wired into the translation task |
| 80 | +in the most recent commit) is the raw-signal layer for that — but there are currently only 3 real |
| 81 | +logged calls, nowhere near enough to learn anything, and no analysis/tuning step exists yet. This |
| 82 | +is worth a genuinely fresh design pass: is "let the agent freely explore, log what it does, learn |
| 83 | +a trim policy later" the right shape at all, given that *generating* enough data to learn from is |
| 84 | +itself expensive under subscription rate limits? Is there a middle path that isn't the rejected |
| 85 | +"hand-picked static filter" but also isn't fully unconstrained exploration? |
| 86 | + |
| 87 | +### 2. Model selection wasn't controlled until well after the fact |
| 88 | + |
| 89 | +`headless_claude.ts` didn't pass `--model` for most of Phase 2. `claude -p` picked on its own — |
| 90 | +a real run came back mostly `claude-haiku-4-5` with one `claude-sonnet-5` call, none of it |
| 91 | +requested by the code. This was only caught by inspecting real generation records, not from any |
| 92 | +code review or test. It's now fixed (`--model` pinned per task type, hardcoded to |
| 93 | +`claude-sonnet-5` for translation), but the fact that the original design's "Model routing |
| 94 | +(learned, not fixed)" section assumed a starting point of *fixed-and-known* model selection, when |
| 95 | +the actual starting point was *uncontrolled*, suggests the plan underspecified how model |
| 96 | +selection works at the CLI boundary. Worth double-checking there isn't a second, similar gap |
| 97 | +elsewhere (e.g. does `--model` reliably pin the model for every internal sub-step the CLI takes, |
| 98 | +or only the "primary" one? `primaryModel()` already has to pick the highest-cost entry out of a |
| 99 | +`modelUsage` map that can contain more than one model per call — that map's existence is itself |
| 100 | +evidence the CLI uses multiple models internally per session, not always the one you asked for.) |
| 101 | + |
| 102 | +### 3. A hidden correctness bug from testing/runtime divergence |
| 103 | + |
| 104 | +`HeadlessClaudeError extends Error`, with an `isRateLimited` field the orchestrator branches on |
| 105 | +to decide "stop the whole run" vs. "skip one candidate." `instanceof HeadlessClaudeError` silently |
| 106 | +returned `false` at this project's tsconfig target (unset → TypeScript defaults to ES3, and |
| 107 | +extending `Error` at that target needs an explicit `Object.setPrototypeOf` fix that wasn't |
| 108 | +present). The bug had **100% unit test coverage of the intended behavior, and the tests passed** — |
| 109 | +because jest runs through babel-jest, which targets something else and doesn't reproduce the bug. |
| 110 | +It was only caught because a real run, after the fix was believed complete, kept logging "Skipping |
| 111 | +[ref]: HeadlessClaudeError: session limit" for every remaining candidate instead of stopping once. |
| 112 | + |
| 113 | +This is a structural risk, not a one-off: this codebase runs the *same TypeScript source* through |
| 114 | +two different toolchains (`ts-node` for real execution, `babel-jest` for tests) with no guarantee |
| 115 | +they agree on language-target-sensitive behavior. Worth asking whether that's an acceptable |
| 116 | +standing risk for this subsystem specifically (which runs unattended, with real cost, against |
| 117 | +real content) or whether it needs its own safeguard — e.g. a lightweight integration test that |
| 118 | +actually runs under `ts-node`, or fixing the tsconfig target properly instead of instance-by-instance |
| 119 | +workarounds. |
| 120 | + |
| 121 | +### 4. Two more bugs only found by running real code, not by mocked tests |
| 122 | + |
| 123 | +- `--limit N` on the CLI was applied *before* freshness filtering, so a small `--limit` against a |
| 124 | + page with existing generated content could slice to only already-fresh candidates and silently |
| 125 | + exit having done nothing — no error, no output, exit code 0. |
| 126 | +- Both `triage_log.ts` and `context_usage_log.ts` originally hardcoded the production log path |
| 127 | + inside their own test files, with a `fs.rmSync` cleanup step on that same path — meaning running |
| 128 | + the test suite after either log had real accumulated data would silently delete it. This |
| 129 | + actually happened once to `context_usage_log.jsonl` mid-session (recovered from the git index |
| 130 | + before it was lost for good). |
| 131 | + |
| 132 | +Both are now fixed, but the pattern is the point: this project leans heavily on dependency-injected |
| 133 | +unit tests with mocked Claude/CLI calls (a deliberate, reasonable choice to avoid spending real |
| 134 | +usage on every test run) — and every real bug found so far was found by *actually running the |
| 135 | +pipeline*, not by that test suite. Worth thinking about whether some cheap, safe form of |
| 136 | +integration testing belongs in this project's standard workflow rather than being ad hoc "let's |
| 137 | +try it for real and see." |
| 138 | + |
| 139 | +### 5. Budget reality under subscription billing |
| 140 | + |
| 141 | +The plan accepted "coarser than a spend cap" as a tradeoff for staying on subscription billing |
| 142 | +rather than metered API billing. In practice this session hit the Claude Code session/usage limit |
| 143 | +**three separate times** across a few hours of real work, each requiring a wait for reset |
| 144 | +(observed resets at varying times — 6:10pm, then 11:10pm, then 4:40am Asia/Jerusalem across the |
| 145 | +session, suggesting a rolling window rather than a fixed daily reset). The rate-limit-stop logic |
| 146 | +now works correctly (see #3), but there's still no proactive pacing — the system finds out it's |
| 147 | +out of budget by hitting the wall, not by tracking spend against a target. `claude -p` has a |
| 148 | +`--max-budget-usd` flag that's documented but still not wired in anywhere. Also unclear: whether |
| 149 | +`--max-budget-usd` is even meaningful under subscription billing (it's phrased as "maximum dollar |
| 150 | +amount to spend on API calls" in the CLI's own help text) — that's worth verifying before relying |
| 151 | +on it as the budget-control mechanism Phase 4 assumed would exist. |
| 152 | + |
| 153 | +### 6. Self-critique's built-in cost multiplier |
| 154 | + |
| 155 | +Every accepted edit costs at least one generate call + one critique call; a rejected first attempt |
| 156 | +doubles that to up to four calls. Given finding #1 (a single generate call can cost $0.73 on its |
| 157 | +own), the multiplier compounds an already-expensive step. This was a deliberate design choice |
| 158 | +("a bounded loop of a couple of extra calls, not a big loss" per the original plan) made before |
| 159 | +real cost data existed. Worth revisiting with that data in hand: is critique-every-time the right |
| 160 | +default, or should it be conditional (e.g. skip critique when the generate call's own tool-use |
| 161 | +pattern suggests low risk) or sampled? |
| 162 | + |
| 163 | +### 7. No review gate exists yet |
| 164 | + |
| 165 | +Everything generated so far has gone straight from the pipeline into git-committed |
| 166 | +`precomputed/ai_additions/` and generation records, with a human (the PR itself) as the only |
| 167 | +review surface. The original plan's Phase 3 ("human review loop," explicitly called out as "the |
| 168 | +exciting part... don't want to waste time on low-judgment tasks before we've proven the project |
| 169 | +works") hasn't been started. Worth deciding whether the current state — real generated Talmud |
| 170 | +translations sitting in a PR, reviewable as a diff — is an acceptable interim review mechanism, or |
| 171 | +whether Phase 3 needs to move up given real content is now being produced. |
| 172 | + |
| 173 | +### 8. Workflow friction (not architectural, but real) |
| 174 | + |
| 175 | +PR #53's squash-merge left the continuing branch's git ancestry out of sync with `base` — the |
| 176 | +branch's `merge-base` pointed at the pre-Phase-1 commit even though its file contents matched. |
| 177 | +Required a manual `git rebase --onto` to fix, caught only because it was explicitly asked about. |
| 178 | +Minor, but worth a process note for whoever manages future PRs against this branch pattern. |
| 179 | + |
| 180 | +## Concrete data worth carrying into a redesign |
| 181 | + |
| 182 | +- Page JSON size breakdown (`Menachot.80a.json`, 476KB total): base segment text 25KB; Mesorat |
| 183 | + Hashas 190KB (42% of commentary); Rashi 87KB; Steinsaltz 36KB; Verses 34KB; Mishneh Torah 25KB; |
| 184 | + Versions 20KB; Mishnah 17KB; Jastrow 17KB; Tosafot 13KB; Steinsaltz In-Depth 8KB; Tosefta 3KB; |
| 185 | + Kessef Mishneh 1KB. |
| 186 | +- Real per-call costs observed: as low as ~$0.09 total (generate + critique, no retry) up to |
| 187 | + $0.73 for a single generate call that went searching the corpus. |
| 188 | +- `--output-format stream-json --verbose` is the mechanism now used to recover both cost/model |
| 189 | + attribution and the full tool-use transcript from a single headless call; the plain `json` |
| 190 | + format only gives the final summary. |
| 191 | +- Fixed per-session overhead (system prompt + tool definitions) was ~9.5–16K tokens on an early |
| 192 | + probe call, before `--allowedTools` was scoped down to `Read,Grep,Glob`. |
| 193 | + |
| 194 | +## Open questions for the next design pass |
| 195 | + |
| 196 | +1. Is "fully agentic, no pre-assembled context" still the right default given #1, or does it need |
| 197 | + a bounded middle ground — and if so, what decides the bound? |
| 198 | +2. How should "learned trimming" actually bootstrap when generating enough data to learn from is |
| 199 | + itself expensive and rate-limited? |
| 200 | +3. Should critique run unconditionally, or be made conditional/sampled given its cost multiplier? |
| 201 | +4. What does "learned model routing" concretely look like given the CLI's own internal |
| 202 | + multi-model behavior (the `modelUsage` map) isn't fully within this code's control? |
| 203 | +5. Is the ts-node/babel-jest target divergence (finding #3) something to fix at the tsconfig |
| 204 | + level, or accept as a standing risk for this subsystem? |
| 205 | +6. Does Phase 3 (human review) need to move up in priority now that real content exists in the |
| 206 | + PR, ahead of scaling up generation volume further? |
| 207 | +7. Is `--max-budget-usd` meaningful under subscription billing, and if so, at what granularity |
| 208 | + should it be applied (per call? per run? per day)? |
0 commit comments