danielmiessler / danielmiessler/LifeOS

MemoryReviewer `--dry-run` calls the model and writes to the health ledger, while its own header promises "no inference"

Open
#2,073 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
19k
Forks
2.5k
Avg merge
8d 17h
Merged PRs (30d)
1

Description

`MemoryReviewer.ts` documents `--dry-run` as **"extract + prompt, no inference"** (line 28 of the file's own header). It calls the model anyway, and it also writes into the two artifacts CortexHealth reads to judge reviewer health. Measured on v7.40.4.

## The mechanism

`review()` threads `dryRun` into exactly one place — `dispatchItems()`:

```ts
const { summary, results } = dispatchItems(parsed.output.items, { dryRun: opts.dryRun });
```

Everything upstream of that runs unconditionally: `inference()` (twice, if the parse retry fires), `writeRunDebug()` (creates `MEMORY/OBSERVABILITY/reviewer-runs//`), and `logRunSummary()` (appends a row to `MEMORY/OBSERVABILITY/reviewer-runs.jsonl`). The flag suppresses the memory writes and nothing else.

## Measurement

Counting run directories and ledger rows around one `--dry-run`, with a synthetic transcript and a counting double at the inference boundary:

| | before | after |
|---|---|---|
| `reviewer-runs/` directories | 254 | **255** |
| `reviewer-runs.jsonl` rows | 257 | **258** |
| calls to `inference()` | — | **1** |

## Why the ledger half is worse than the cost

`CortexHealth.reviewerEvidence()` treats a run directory newer than the latest ledger row as an orphaned run, and past `reviewerRunGraceMs` reports it as `timed-out` with a fabricated `consecutiveFailures` streak:

```ts
if (latestDir && Number.isFinite(latestDirMs) && (!Number.isFinite(latestRowMs) || latestDirMs > latestRowMs)) {
...
if (nowMs - latestDirMs > thresholds.reviewerRunGraceMs) return { status: "timed-out", ... };
}
```

So the "safe" command can make the health check report a timeout that never happened, and every dry run inflates the denominator that `MemoryFixesCheck`, `MemoryInsights` and `MemoryStatus` read. A flag that promises to touch nothing, and is therefore reached for precisely when someone is being careful, is the worst place to put an unlogged side effect.

## Suggested shape

Split the two things the flag was doing:

- `--dry-run` returns right after the prompts are built — no inference, no run directory, no ledger row — and prints what it would have sent (transcript, exchange count, prompt sizes).
- a separate `--preview` keeps today's useful behaviour (call the model, show the parsed items, write no memories), with its artifacts in their own directory so they can never be mistaken for a run, and no ledger row.

## Sibling in the same release

`KnowledgeDistill.ts run --headless --dry-run` spawns `Inference.ts --level high --timeout 240000` **before** it checks the flag. There the spend is arguably inherent — the digest preview *is* the synthesis — so the fix is the usage text saying so, not a return. Worth a line either way, since today the cost is discovered by paying it.

Happy to send a PR for either or both.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in MemoryReviewer.ts, reading its header and review()/dispatchItems() flow, then trace writeRunDebug(), logRunSummary(), and CortexHealth.reviewerEvidence(). Confirm the selected dry-run or preview behavior prevents unintended inference and health-ledger artifacts, while preserving the intended displayed preview information; consider the related KnowledgeDistill.ts and Inference.ts usage text separately.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
ai, cli, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.