Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
359 lines
13 KiB
Markdown
359 lines
13 KiB
Markdown
# M5c-2 task 09: `recall` asks Honcho, outside the state lock
|
|
|
|
**Branch:** `m5c2`. Check it: `git branch --show-current` prints `m5c2` and `git status --short`
|
|
is empty; otherwise stop. Never switch branch.
|
|
**Commit subject:** `memoryd: recall asks Honcho after the local index, outside the state lock`
|
|
**Log row:** the Task cell is `M5c2/09-memoryd-recall-honcho`.
|
|
|
|
## Goal
|
|
|
|
Task 08 wrote `recall`'s Honcho part (`honcho/search.rs`): `search` asks every source at once,
|
|
each with its own limit, and `answer` makes the answer from the local index's items and
|
|
Honcho's. This task makes `memoryd`'s `recall` use it (M5c spec, section 8).
|
|
|
|
- **With `[honcho]`**, a `recall` call:
|
|
1. takes the state lock, runs `recall::local` (task 07: the search, the recall log, the local
|
|
items and their label), and releases the lock;
|
|
2. then, with no lock held, runs the Honcho part and makes the answer: `search::Reading::recall`,
|
|
a new method that is `search` then `answer`.
|
|
|
|
So a source that hangs for its 5 s holds up no `memory` call, no memory block and no other
|
|
`recall`: before this task, `call` held the lock for the whole of any call.
|
|
- **Without `[honcho]`**, `recall` is exactly as before: `recall::recall` under the lock.
|
|
- **`memory`** calls are unchanged: under the lock for the whole call.
|
|
- **At start**, with `[honcho]`, `start` keeps what `recall` reads, `search::Reading { readers,
|
|
owner, wait }`: the readers task 05 built (`opened.readers`, taken before the worker's thread
|
|
takes the rest of `Opened`), the owner's peer from `[honcho]` (`honcho.owner`), and
|
|
`search::SOURCE_WAIT` (5 s).
|
|
|
|
`serve.rs` is near the 500-line limit (495 lines after this task), so `Reading` and its method go
|
|
into `search.rs`, not `serve.rs`.
|
|
|
|
## Every place that changes
|
|
|
|
This is the complete list.
|
|
|
|
| Place | What happens there |
|
|
|---|---|
|
|
| `crates/memoryd/src/honcho/search.rs` | `Reading` and `Reading::recall` (edit S1) |
|
|
| `crates/memoryd/src/serve.rs` | a `use` line; `Memoryd::reading`; `start` builds it; `call` and `recall` (edits E1 to E7) |
|
|
| `crates/memoryd/tests/honcho_recall_serve.rs` | new, given |
|
|
| everything else | **no change**: not `recall.rs`, not `start.rs`, not the worker, no `Cargo.toml` |
|
|
|
|
## Files
|
|
|
|
- Copy, from `docs/plans/M5c2/files/` (the same path under it): the new test
|
|
`crates/memoryd/tests/honcho_recall_serve.rs`.
|
|
- Modify: `crates/memoryd/src/honcho/search.rs`, `crates/memoryd/src/serve.rs`,
|
|
`docs/implementer-log.md`.
|
|
- Do not touch: any other file.
|
|
|
|
## Facts you need
|
|
|
|
- **`serve.rs` today.** `start` opens the Honcho side (`worker::open`, returning `Opened`) when
|
|
`config.honcho_config()?` is `Some(honcho)`, and later moves `opened` into the worker's thread
|
|
(`worker::spawn`). `Opened::readers` is an `Arc<Readers>` (task 05). `call` takes the state
|
|
lock (`self.lock()`) first, then runs `recall` or `memory` with it.
|
|
- **Taking the lock for a block only**: `let local = { let state = self.lock(); … };` releases
|
|
the lock at the block's closing brace, when `state` is dropped. The Honcho part runs after it.
|
|
- **`let … else`**: `let Some(reading) = &self.reading else { …; return …; };` runs the `else`
|
|
block (which must return) when there is no `[honcho]`.
|
|
- **What `recall` answers with `[honcho]`** (task 08's `answer`): the local items, then Honcho's
|
|
up to `k` in all, numbered after them, then any `{source}: unavailable` line;
|
|
`(no matches for "…")` when there is nothing. A failed or late source is logged by
|
|
`search`, never with the query.
|
|
- **The given test** `honcho_recall_serve.rs` (4 tests; about 5 s), through `memoryd`'s `brokerd`
|
|
socket, against the fake Honcho (task 08's version):
|
|
- a local hit, then `boxmaker1`'s (labelled trusted, among them a conclusion about the owner's
|
|
peer, `kyle`) and the fleet's; the answer untrusted because a fleet hit is shown (T58); the
|
|
agent's own workspaces never searched; with the fleet missing, no fleet hit and a trusted
|
|
answer;
|
|
- while the fleet hangs, a `memory` call answered within 2 s and the `recall` still waiting;
|
|
then the `recall` answered with `fleet: unavailable`, and the line logged;
|
|
- nothing found anywhere: `(no matches for "zebra")`;
|
|
- without `[honcho]`: the local answer exactly as before, and nothing logged.
|
|
- The items the edits use. This table is complete.
|
|
|
|
| Item | Where |
|
|
|---|---|
|
|
| `search::search`, `search::answer`, `search::SOURCE_WAIT` (task 08) | S1, E4 |
|
|
| `recall::local`, `recall::recall` (task 07) | E7 |
|
|
| `Arc::clone` | E4 |
|
|
|
|
## The edits, exactly
|
|
|
|
Nothing else in these files changes. Each quoted "before" block occurs exactly once in its file
|
|
when its turn comes (make them in order). Keep the fields in the order shown.
|
|
|
|
### `crates/memoryd/src/honcho/search.rs`
|
|
|
|
**S1.** The line
|
|
|
|
```rust
|
|
/// What the Honcho part found.
|
|
```
|
|
|
|
becomes
|
|
|
|
```rust
|
|
/// What `recall` reads in Honcho: the sources, the owner's peer, and each source's limit
|
|
/// (`SOURCE_WAIT` in `memoryd`).
|
|
pub struct Reading {
|
|
pub readers: Arc<Readers>,
|
|
pub owner: String,
|
|
pub wait: Duration,
|
|
}
|
|
|
|
impl Reading {
|
|
/// `recall`'s answer from the local index's items and Honcho's: `search`, then `answer`.
|
|
pub fn recall(&self, local: Local, args: &RecallArgs, log: &Log) -> Recalled {
|
|
let searched = search(&self.readers, &self.owner, args, self.wait, log);
|
|
answer(local, &args.query, args.k, searched)
|
|
}
|
|
}
|
|
|
|
/// What the Honcho part found.
|
|
```
|
|
|
|
### `crates/memoryd/src/serve.rs`
|
|
|
|
**E1.** Add one line. The line
|
|
|
|
```rust
|
|
use crate::honcho::report;
|
|
```
|
|
|
|
becomes
|
|
|
|
```rust
|
|
use crate::honcho::report;
|
|
use crate::honcho::search;
|
|
```
|
|
|
|
**E2.** In `struct Memoryd`, the lines
|
|
|
|
```rust
|
|
mirror: Option<Mirror>,
|
|
log: Log,
|
|
}
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
mirror: Option<Mirror>,
|
|
/// With `[honcho]`: what `recall` reads beside the local index, outside `state`.
|
|
reading: Option<search::Reading>,
|
|
log: Log,
|
|
}
|
|
```
|
|
|
|
**E3.** In `start`, the lines
|
|
|
|
```rust
|
|
let opened = match config.honcho_config()? {
|
|
Some(honcho) => Some(worker::open(&config, &honcho, &log)?),
|
|
None => None,
|
|
};
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
let (opened, owner) = match config.honcho_config()? {
|
|
Some(honcho) => (
|
|
Some(worker::open(&config, &honcho, &log)?),
|
|
honcho.owner.clone(),
|
|
),
|
|
None => (None, String::new()),
|
|
};
|
|
```
|
|
|
|
**E4.** In `start`, the lines
|
|
|
|
```rust
|
|
let (honcho, mirror) = match opened {
|
|
Some(opened) => {
|
|
let sync = Arc::clone(&opened.sync);
|
|
let mirror = opened.mirror();
|
|
worker::spawn(opened, &config, Arc::clone(&wake), Arc::clone(&log))?;
|
|
(Some(sync), Some(mirror))
|
|
}
|
|
None => (None, None),
|
|
};
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
let (honcho, mirror, reading) = match opened {
|
|
Some(opened) => {
|
|
let sync = Arc::clone(&opened.sync);
|
|
let mirror = opened.mirror();
|
|
let reading = search::Reading {
|
|
readers: Arc::clone(&opened.readers),
|
|
owner,
|
|
wait: search::SOURCE_WAIT,
|
|
};
|
|
worker::spawn(opened, &config, Arc::clone(&wake), Arc::clone(&log))?;
|
|
(Some(sync), Some(mirror), Some(reading))
|
|
}
|
|
None => (None, None, None),
|
|
};
|
|
```
|
|
|
|
**E5.** In `start`, the lines
|
|
|
|
```rust
|
|
wake,
|
|
mirror,
|
|
log,
|
|
}),
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
wake,
|
|
mirror,
|
|
reading,
|
|
log,
|
|
}),
|
|
```
|
|
|
|
**E6.** The lines
|
|
|
|
```rust
|
|
/// A decided call from `brokerd`: `recall` or `memory`, under the one lock. `Failed` carries the
|
|
/// sentence for the model.
|
|
fn call(&self, call: MemoryCall) -> ToolResponse {
|
|
let state = self.lock();
|
|
let result = match call.request.tool.as_str() {
|
|
bxcore::memory::RECALL => self.recall(&state, &call),
|
|
bxcore::memory::MEMORY => self.memory(&state, &call),
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
/// A decided call from `brokerd`: `recall` or `memory`. `memory` runs under the one lock;
|
|
/// `recall` takes it for the local index only (`recall`). `Failed` carries the sentence for
|
|
/// the model.
|
|
fn call(&self, call: MemoryCall) -> ToolResponse {
|
|
let result = match call.request.tool.as_str() {
|
|
bxcore::memory::RECALL => self.recall(&call),
|
|
bxcore::memory::MEMORY => self.memory(&self.lock(), &call),
|
|
```
|
|
|
|
**E7.** The lines
|
|
|
|
```rust
|
|
/// `recall`: search, labelled by what it found.
|
|
fn recall(&self, state: &State, call: &MemoryCall) -> Result<Recalled, String> {
|
|
let args: RecallArgs = match serde_json::from_str(&call.request.arguments) {
|
|
Ok(args) => args,
|
|
Err(e) => return Err(format!("recall's arguments: {e}")),
|
|
};
|
|
recall::recall(
|
|
&state.index,
|
|
&self.store,
|
|
&args,
|
|
call.request.session.as_str(),
|
|
Timestamp::now(),
|
|
)
|
|
}
|
|
```
|
|
|
|
become
|
|
|
|
```rust
|
|
/// `recall`: search, labelled by what it found. Without `[honcho]`, the local index under the
|
|
/// lock, as before. With it, the local index under the lock, then, with the lock released,
|
|
/// Honcho's sources (M5c spec, section 8): a source that hangs holds up no other request.
|
|
fn recall(&self, call: &MemoryCall) -> Result<Recalled, String> {
|
|
let args: RecallArgs = match serde_json::from_str(&call.request.arguments) {
|
|
Ok(args) => args,
|
|
Err(e) => return Err(format!("recall's arguments: {e}")),
|
|
};
|
|
let session = call.request.session.as_str();
|
|
let Some(reading) = &self.reading else {
|
|
let state = self.lock();
|
|
return recall::recall(&state.index, &self.store, &args, session, Timestamp::now());
|
|
};
|
|
let local = {
|
|
let state = self.lock();
|
|
recall::local(&state.index, &self.store, &args, session, Timestamp::now())?
|
|
};
|
|
Ok(reading.recall(local, &args, &self.log))
|
|
}
|
|
```
|
|
|
|
## Steps
|
|
|
|
- [ ] **1. Copy the given test.** From the repository root, one command:
|
|
|
|
```sh
|
|
cp docs/plans/M5c2/files/crates/memoryd/tests/honcho_recall_serve.rs crates/memoryd/tests/
|
|
```
|
|
|
|
- [ ] **2. See it fail.** `cargo test --no-fail-fast -p memoryd --test honcho_recall_serve`, with
|
|
a shell timeout of 600000 ms: it compiles, and two tests fail:
|
|
`a_fleet_hit_makes_the_answer_untrusted` (no Honcho hit is shown yet) and
|
|
`a_hanging_source_holds_up_no_memory_call` (the fleet is never searched: ``the fleet was never
|
|
searched``). The other two pass already.
|
|
- [ ] **3. The edits.** Make edits S1 and E1 to E7, in order. `cargo check -p memoryd` prints no
|
|
error and no warning.
|
|
- [ ] **4. The tests.** With a shell timeout of 600000 ms, every test passes: 4 in
|
|
`honcho_recall_serve`, and as before 12 in `honcho_search`, 6 in `honcho_serve`, 8 in `serve`
|
|
and 5 in `serve_sanitize`:
|
|
|
|
```sh
|
|
cargo test -p memoryd --test honcho_recall_serve --test honcho_serve --test serve \
|
|
--test serve_sanitize --test honcho_search
|
|
```
|
|
- [ ] **5. Check.** Two commands:
|
|
- `git diff --stat` lists exactly `crates/memoryd/src/honcho/search.rs` and
|
|
`crates/memoryd/src/serve.rs`.
|
|
- `wc -l crates/memoryd/src/serve.rs` prints 495.
|
|
- [ ] **6. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms.
|
|
Expected last line: `gate: ok`. The test count is the passed total of `cargo test --workspace`
|
|
plus one, for the ignored `end_to_end` test that the gate runs on its own: **1434** after this
|
|
task, 1430 before it; the 4 new ones are the tests of `honcho_recall_serve.rs`. To count, add
|
|
up the `N passed` of every `test result:` line the gate's `cargo test --workspace` printed,
|
|
and add 1. Never run the gate again only to count.
|
|
- [ ] **7. Log and commit.** Add your row to `docs/implementer-log.md`; its Task cell is
|
|
`M5c2/09-memoryd-recall-honcho`. Stage with
|
|
`git add crates/memoryd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` says.
|
|
`Cargo.lock` does not change in this task; the `git add` is harmless.
|
|
|
|
## Done when
|
|
|
|
- `make gate` prints `gate: ok` with 1434 tests.
|
|
- `git diff HEAD~1 --stat` lists only `search.rs`, `serve.rs`, `honcho_recall_serve.rs` and
|
|
`docs/implementer-log.md`.
|
|
|
|
## Stop and report if
|
|
|
|
- The branch check at the top fails.
|
|
- A "before" block is not in its file exactly as shown when its turn comes, or is there more
|
|
than once.
|
|
- Step 2 shows a different result: a compile error, or other tests failing.
|
|
- `cargo check` in step 3 prints an error or a warning. Quote it, and stop.
|
|
- A test fails in step 4. Do not change a test: stop, and give the failing tests' names and
|
|
their messages.
|
|
- The fix seems to need a change anywhere else: `recall.rs`, `start.rs`, `read.rs`, the worker,
|
|
a configuration key for the limit. It does not; stop and say why you think so.
|
|
- `serve.rs` passes 500 lines, or the gate's line check fails.
|
|
- A test outside the given file fails in the gate: check once whether it fails without your
|
|
change (`git stash -u`, run it, `git stash pop`), then stop and report both results.
|
|
- The gate passes with a count other than 1434.
|
|
- A comment in an edit does not fit the code under it. Quote it and stop.
|
|
|
|
## What the given tests pin
|
|
|
|
Each of these changes to the finished code makes at least one given test fail (8 tried, all
|
|
caught): the state lock held across the Honcho part; Honcho not asked though configured; no
|
|
`Reading` built at start; the agent's peer passed as the owner's; the path without `[honcho]`
|
|
going through the Honcho answer; `Reading::recall` dropping Honcho's label; a zero limit; the
|
|
local items left out of the answer.
|