📝 Add orbit research tree, goal, and session journal
Research tree on using the orbit code-graph CLI for review in Rust projects: the Python/Django field guide recorded as phase 0, orbit 0.133 capabilities on spacecraft and dirigent, fit with Spire's review stage, and tested review recipes (changed definitions, blast radius, blindness preflight). Includes the orbit-skill goal and the Igor session journal.
This commit is contained in:
@@ -0,0 +1,497 @@
|
||||
---
|
||||
phase: 2
|
||||
stream: review-workflows
|
||||
date: 2026-09-29
|
||||
orbit-version: 0.133.0
|
||||
depends-on: phase-1-rust-capabilities.md, phase-1-review-fit.md, phase-0-field-guide-python-django.md
|
||||
test-beds: spacecraft (2 ranges), dirigent (5 ranges), all historical, indexed from detached worktrees into per-review isolated DBs
|
||||
status: findings
|
||||
---
|
||||
|
||||
# Phase 2: Review workflows with orbit + grep (Rust)
|
||||
|
||||
Legend: **[V]** = verified here with command output and a ground-truth comparison. **[I]** = inferred, not tested directly.
|
||||
|
||||
Scripts, SQL and rules files are in `~/.cache/orbit-probe/recipes/` (full list in §11). Raw outputs for every range are in `~/.cache/orbit-probe/out/`. Nothing was compiled. The shared DB `~/.gitlab/orbit/graph.duckdb` was not written: its sha256 was the same before and after the run.
|
||||
|
||||
## TL;DR
|
||||
|
||||
- **Orbit gives a reviewer a structural map of a Rust diff that the diff text does not.** It lists which definitions were added, removed, changed in signature or touched in body; where their callers and importers are; which trait implementors must follow a trait change; and whether new crate edges appeared. A full pass indexes base and tip into a throwaway DB and runs six scripts. It takes **about 30 s** and prints **about 16 KB**, compared with a 56–411 KB raw diff. [V]
|
||||
- **Within the code orbit can see, the changed-definitions map is exact:** precision 100 % and recall 100 % across 7 ranges. **But orbit cannot see much of Rust:**
|
||||
- Of all item declarations the diffs added or removed, 27–100 % were visible depending on the range. **Every miss** was in `#[cfg(test)]` code, a `cfg(feature)` region, or a file gated at a parent `mod`.
|
||||
- The preflight measured **6 % to 71 % of changed lines** as invisible to orbit. dirigent's `server` feature hides whole connector modules.
|
||||
- **Run the blindness preflight first. It decides how much to trust everything after it.** [V]
|
||||
- **Blast radius: one level down, orbit misses half; one level up, it misses little.** Recall at **definition level** was **49 %** (129 of 265 grep-confirmed dependents). At **file level**, callers plus importers reached **93 %** (138/148). The gaps:
|
||||
- `cfg(test)` code: 36
|
||||
- enum variant literals and multi-line patterns: 32
|
||||
- Bevy systems passed as fn refs: 21
|
||||
- gated files: 17
|
||||
- `Type::default()`-style calls: 6
|
||||
- `rsx!` bodies: 4
|
||||
- `dyn` receivers: 2
|
||||
|
||||
Report orbit's callers as a **lower bound**. Close each gap with the grep named in its class. [V]
|
||||
- **The review findings that no plain `git diff` shows:**
|
||||
- **Doc drift:** a new crate edge `warpspace_replay → warpspace_sync_core` contradicts the dependency graph in AGENTS.md.
|
||||
- **Re-export shims:** 8 of 8 removed duplicate types were correctly traced to `pub use` shims that keep the old paths alive for 1–13 importers.
|
||||
- **Hidden trait implementor:** a cfg-gated `MockBackend` of a changed trait. Orbit cannot see it; the grep fallback in the trait recipe does.
|
||||
- **Stop signal:** a branch where 71 % of changed lines are feature-gated. There the preflight says to stop relying on orbit. [V]
|
||||
- **New facts about orbit:**
|
||||
- **Orbit indexes the working tree, including uncommitted and untracked files, but labels the rows with HEAD's sha.** [V]
|
||||
- `repo-map api` accepts **directory** prefixes only. A file path silently returns nothing. [V]
|
||||
- No import rows are recorded for `use` inside `#[cfg(test)] mod` blocks (0 of 621) or inside fn bodies (mostly none). Inline paths such as `other_crate::x::Y` never become import rows. [V]
|
||||
- **Feedback log:** one JSONL line per orbit query that informed a decision. Written by a tested helper, `fb.sh`, and stored by default outside git at `~/.local/share/orbit-skill/feedback.jsonl` (§9).
|
||||
|
||||
---
|
||||
|
||||
## 1. Test beds and ranges
|
||||
|
||||
| id | repo | range (base..tip) | what it is | .rs files | +/− lines |
|
||||
|---|---|---|---|---|---|
|
||||
| sc-replay | spacecraft | `21cdd1fd..bfbbcd96` (`45c50f07^..bfbbcd96`) | replay feature FL-1..3 + refactor, 13 commits | 72 | +2245/−361 |
|
||||
| sc-unify | spacecraft | `00325961..8df804e1` (`a8711df5^..8df804e1`) | "unify duplicate types / delete dead code" series | 39 | +364/−1361 |
|
||||
| dg-tokens | dirigent | `9e9374be..d3f99fb7` (merge PR #28) | session token stats | 17 | +1043/−32 |
|
||||
| dg-acp | dirigent | `0c40e871..5de6c726` (merge-base(main)..branch) | W1c live ACP branch (typed updates, generic `SessionUpdate<Id>`) | 58 | +4331/−2020 |
|
||||
| dg-archivist | dirigent | `313c0f11..22034c02` (merge) | reading pane + archivist redesign | 32 | +5033/−170 |
|
||||
| dg-trash | dirigent | `300f4319^..300f4319` | adds `ArchiveBackend::trash_session` | 17 | +1153/−120 |
|
||||
| dg-move | dirigent | `cd4e8f70^..cd4e8f70` | adds `delete_session_if_unchanged`, removes `log_stamp` from the trait | 8 | +324/−142 |
|
||||
|
||||
Every index used `-t 4`: `skipped_files=0` and `errored_files=0` on all 16 indexes. [V]
|
||||
|
||||
## 2. Per-review setup (shared by all recipes)
|
||||
|
||||
```bash
|
||||
R=~/.cache/orbit-probe/recipes
|
||||
$R/review-setup.sh ~/Projects/dirigent 300f4319^ 300f4319 dg-trash > out/dg-trash.env
|
||||
# -> DB=~/.cache/orbit-probe/dg-trash.duckdb BASE_WT=…/worktrees/dg-trash-base TIP_WT=…/worktrees/dg-trash-tip BASE_SHA TIP_SHA REPO
|
||||
...recipes...
|
||||
$R/review-teardown.sh ~/Projects/dirigent dg-trash # git worktree remove + prune + rm DB
|
||||
```
|
||||
|
||||
- **Both commits go into one isolated DB.** `orbit sql --repo <worktree> --db <db>` scopes each query to one commit, so recipes query base and tip separately and join them in Python on `(fqn, kind)`. Definition IDs differ between commits. [V]
|
||||
- **Time:** about 6 s per index with `-t 4`, plus worktree checkout. `review-pass.sh` (all six recipes) ran in **30 s** wall time on dg-tokens. [V]
|
||||
- **Orbit indexes the working tree, not the commit.** I appended a fn to a tracked file and created an untracked `.rs` file in a scratch worktree, then indexed it into a throwaway DB. Both appeared: `orbit_probe_dirty_marker` and `zz_probe_untracked.rs`. The manifest still said `commit_sha=cd4e8f70` (HEAD). [V] This has two consequences:
|
||||
- (a) An index labelled with sha X proves nothing if the tree was dirty. The preflight checks `git status --porcelain`.
|
||||
- (b) An **uncommitted** diff, for example Athanor output before commit, can be reviewed by indexing the live checkout into an isolated `--db` as "tip". [I: tested only in a scratch worktree, not on a live checkout] Phase 1's statement that "uncommitted edits are not visible" is corrected.
|
||||
- **Do not reuse the shared DB for review.** Its per-repo row is replaced by whichever agent indexed last, and is often stale (phase 1).
|
||||
|
||||
---
|
||||
|
||||
## 3. Recipe 1: Changed definitions
|
||||
|
||||
**Answers:** which definitions the diff added, removed, moved, changed in signature, or touched only in body, with a one-line signature delta and field/variant membership changes. Also which changed lines no definition covers; that tally feeds recipe 5.
|
||||
|
||||
**Command:** `python3 $R/defdiff.py out/<id>.env --json out/<id>.defdiff.json`
|
||||
|
||||
**How it works:**
|
||||
1. `git diff -U0 -M base tip -- '*.rs'` gives changed line numbers on both sides.
|
||||
2. Each line maps to its innermost orbit definition. Fields and variants roll up to their parent type; `Module` is used only if nothing smaller covers the line.
|
||||
3. Definitions are matched across commits on `(fqn, kind)`, same file first, following renames.
|
||||
4. The signature is re-read from the source: the declaration up to `{` or `;`, skipping docs and attributes. It is compared as normalised text.
|
||||
5. Children of an added or removed type collapse into `(+N members)`.
|
||||
6. Uncovered lines are classified as cfg-test, cfg-feature, cfg-other, file-has-no-defs, import, impl-header/brace or blank/comment.
|
||||
|
||||
A single-hunk manual variant is in `sql/hunk-to-defs.sql`.
|
||||
|
||||
**Sample output** (sc-replay, excerpt):
|
||||
```
|
||||
crates/warpspace_debug/src/bin/ui_main.rs
|
||||
S Function spawn_embedded_server_on_menu L234-273
|
||||
sig: …on<Res<EmbeddedServer>>, [--]{+replay_path: Option<Res<ReplayPath>>, +})
|
||||
+ Struct ReplayPath L47-49
|
||||
~ Struct Args L13-45 29->33 lines members +replay
|
||||
crates/warpspace_domains/src/orders/core.rs
|
||||
~ Enum orders::core::OrderOrigin L155-173 15->19 lines members +External.peer_debug +External.player -External.0
|
||||
src/sync/game_record.rs
|
||||
S Constant sync::game_record::FORMAT_VERSION L413-418
|
||||
sig: …st FORMAT_VERSION: u32 = [-5-]{+6+};
|
||||
```
|
||||
The `OrderOrigin::External` change from a tuple variant to a struct variant is visible at a glance. It forced edits in 35 test files, none of which orbit can see (recipe 5).
|
||||
|
||||
**Measured against ground truth** (`gt_items.py`). Ground truth is a regex extraction of item declarations (`fn|struct|enum|trait|const|static|type|mod|macro_rules!`) from both file versions; the table compares added and removed items per file:
|
||||
|
||||
| range | added: text / orbit / recall | removed: text / orbit / recall | precision | every miss is in… |
|
||||
|---|---|---|---|---|
|
||||
| sc-replay | 87 / 78 / 90 % | 3 / 1 / 33 % | 100 % | cfg-test (5), file with no defs (6: `warpspace_testing/src/tests/*`) |
|
||||
| sc-unify | 5 / 5 / 100 % | 79 / 71 / 90 % | 100 % | cfg-test (8) |
|
||||
| dg-tokens | 40 / 15 / 38 % | 0 | 100 % | cfg-test (23), cfg-feature (2) |
|
||||
| dg-acp | 152 / 41 / 27 % | 32 / 2 / 6 % | 100 % | cfg-test (109), file gated by `server` (32) |
|
||||
| dg-archivist | 189 / 104 / 55 % | 0 | 100 % | gated file (44), cfg-test (40), cfg-feature (1) |
|
||||
| dg-trash | 52 / 37 / 71 % | 1 / 0 | 100 % | cfg-test (13), cfg-feature (3) |
|
||||
| dg-move | 14 / 5 / 36 % | 5 / 4 / 80 % | 100 % | cfg-test (6), gated file (3), cfg-feature (1) |
|
||||
|
||||
- **Precision is 100 % everywhere, and recall on orbit-visible code is 100 %.** Every missed item fell into a cfg or gated category. [V]
|
||||
- **Signature detection** was checked against fn declaration lines that change on both sides of the diff:
|
||||
- Every one was either reported as `S`, or sat in gated/test code (dg-acp: 9 in `server`-gated or test code).
|
||||
- Two single-line fns (`new`, `world`) whose declaration line changed only in the body were correctly reported as body-only (`~`). [V]
|
||||
- **Moves** (`>`, same fqn in another file) did not occur in any range. The logic is untested. [I] Moves across crates change the fqn and show as `-`/`+`. The script annotates a `+` with "same name removed in …" when the name and kind match.
|
||||
|
||||
**Blind spots:**
|
||||
- Everything under cfg-off: all features off, `test` off, `debug_assertions` off.
|
||||
- Files gated at a parent `mod`.
|
||||
- Changes to `impl` headers or where-clauses. There is no Impl node, so the line is unattributed.
|
||||
- Changes to `#[derive]` or attributes count as touching the item, because `start_line` includes attributes.
|
||||
- Signature text over 12 lines is cut.
|
||||
|
||||
**Output size:** 2.5–12.7 KB, which is **3–8 % of the raw diff** (sc-replay 12.7 KB against 214 KB; dg-acp 12.3 KB against 411 KB). About 1–3.5k tokens. Runtime about 1.5 s. [V]
|
||||
|
||||
## 4. Recipe 2: Blast radius
|
||||
|
||||
**Answers:** for each changed definition (`S`, `-`, `>`, and `~` with member changes), who depends on it:
|
||||
- CALLS callers. For a type, this includes callers of its methods, associated fns and variant constructors.
|
||||
- Importing files.
|
||||
- EXTENDS implementors, for traits.
|
||||
|
||||
Dependents are split into **in-diff** and **OUTSIDE diff**. The outside ones are what a reviewer has to check. For removed definitions it also reports:
|
||||
- whether a same-name definition survives elsewhere, and
|
||||
- whether the tip still imports the old path. If so, is the path **kept alive by a re-export**, or does it dangle?
|
||||
|
||||
**Command:** `python3 $R/blast.py out/<id>.env out/<id>.defdiff.json [--measure]`. Standalone SQL: `sql/dependents.sql`, `sql/importers.sql`.
|
||||
|
||||
**Sample output** (dg-acp, then sc-unify):
|
||||
```
|
||||
~ Enum conversation::MessagePart [crates/dirigent_protocol/src/conversation.rs] callers=16 outside=12 importers=37 outside=17
|
||||
<- crates/dirigent_protocol/src/merge.rs:447 apply_to_message
|
||||
<- crates/dirigent_archivist/src/record_upgrade.rs:511 unreadable_placeholder
|
||||
… 6 more outside callers
|
||||
- Struct info::UnitInfo [crates/warpspace_combat/src/info.rs] callers=0 outside=0 importers=9 outside=9
|
||||
= same-name Struct survives at tip in: crates/warpspace_core/src/info_vocab.rs
|
||||
= old path '…info::UnitInfo' kept alive by re-export at crates/warpspace_combat/src/lib.rs:17 (8 importers use it)
|
||||
```
|
||||
|
||||
**Measured** (`--measure`). Ground truth was `git grep` at the same commit, with these rules:
|
||||
- Method names are matched as `.name`/`::name`, associated fns as `Type::name`.
|
||||
- A site counts only if its file can see the name: it imports the name, glob-imports the owner module, is the defining file, or uses a qualified `owner::name`.
|
||||
- Import lines, type mentions, impl headers, pattern matches and const reads are excluded. These produce no edge by design, and are not "callers".
|
||||
- Names defined more than once at that commit were skipped as ambiguous (66 targets, mostly sc-unify's duplicates).
|
||||
|
||||
| range | dependents (grep ∪ orbit) | orbit found | def-level recall | file-level (callers ∪ importers) |
|
||||
|---|---|---|---|---|
|
||||
| sc-replay | 41 | 15 | 37 % | 27/31 = 87 % |
|
||||
| sc-unify | 13 | 5 | 38 % | 10/11 = 91 % |
|
||||
| dg-tokens | 5 | 2 | 40 % | 2/2 |
|
||||
| dg-acp | 179 | 91 | 51 % | 80/84 = 95 % |
|
||||
| dg-archivist | 19 | 8 | 42 % | 13/14 |
|
||||
| dg-trash | 3 | 3 | 100 % | 3/3 |
|
||||
| dg-move | 5 | 5 | 100 % | 3/3 |
|
||||
| **total** | **265** | **129** | **49 %** | **138/148 = 93 %** |
|
||||
|
||||
**Why orbit missed a dependent, with the grep that closes each gap** (136 misses, all [V]):
|
||||
|
||||
| gap class | n | example | fallback |
|
||||
|---|---|---|---|
|
||||
| inside `#[cfg(test)]` | 36 | `game_record.rs:2437 RecordCounters::default()` | `git grep -nw Name` limited to test modules; read the test module |
|
||||
| enum variant literal / multi-line match pattern | 32 | `SessionUpdateVariant::AgentMessageChunk(chunk)` | `git grep -nE 'Enum::\w+'`. For an **added variant**, find every `match` on the enum: exhaustiveness is invisible to orbit |
|
||||
| fn passed by reference (Bevy system, callback) | 21 | `app.add_systems(FixedFirst, conductor_gate_system);` | `git grep -nw fn_name`. For a Bevy system a changed signature is harmless, so ask "is it still registered?" |
|
||||
| file gated at parent `mod` / tests dir | 17 | `connectors/opencode.rs` (`server` feature) | read the file; `git grep` |
|
||||
| `Type::default()`, derive/trait associated calls | 6 | `RecordCounters::default()` | `git grep -n 'Type::'` |
|
||||
| other (struct literal `Type { … }`, return type in signature) | 12 | `let runner = HeadlessRunner {` | `git grep -nw Type` |
|
||||
| inside a multi-line `rsx!` body | 4 | `let title = format_token_title(` in a `rsx!{}` | `git grep -nw` |
|
||||
| cfg-other / cfg-feature | 6 | `matches!(&err, ArchivistError::LogUnreadable …)` | read the region |
|
||||
| method on an unresolved receiver (dyn/field/closure) | 2 | `.translate_notification(…)` | `git grep -n '\.method('` |
|
||||
|
||||
- **Orbit-only dependents** (19 in dg-acp, 7 in sc-replay) are real. For a changed type, they are callers of its methods, which a grep for the type name cannot find. Examples: `HeadlessRunner <- fast_forward_to`, `ArchivistError <- backfill_connector_sessions`. For types, orbit and grep complement each other. [V]
|
||||
- **The re-export check was 8/8 correct** on sc-unify. All 8 removed types whose old path the tip still imports are kept alive by `pub use`. I checked 5 by hand, for example `input/runtime.rs:14 pub use warpspace_core::input_vocab::{ActionState, Actions, InputRuntime};`. A naive "name still imported" check would have flagged all 8 as dangling. [V]
|
||||
|
||||
**Output size:** brief mode (no `--measure`) is 0.9–5 KB for feature ranges and 14 KB for sc-unify's 48 removal targets. Runtime about 5 s. [V]
|
||||
|
||||
## 5. Recipe 3: Trait-impl coverage
|
||||
|
||||
**Answers:** for each trait the diff touches (the trait itself, or a method whose parent is a trait):
|
||||
- its methods at tip, and which were added, removed or changed in signature;
|
||||
- every implementor, from three sources:
|
||||
- `E`: EXTENDS; only same-file impls.
|
||||
- `G`: `git grep 'impl … Trait … for Type'`. Text, so it also sees gated and test impls.
|
||||
- `H`: method-name overlap of at least 60 %.
|
||||
- per implementor, how many trait methods it defines and which the diff touched;
|
||||
- for implementors orbit cannot see, a **text check that the new method exists in the impl file**.
|
||||
|
||||
**Command:** `python3 $R/traitcov.py out/<id>.env out/<id>.defdiff.json [--trait Name] [--all-traits]`. Standalone SQL: `sql/implementors.sql` (E ∪ H).
|
||||
|
||||
**Sample output** (dg-trash):
|
||||
```
|
||||
Trait dirigent_archivist::backend::traits::ArchiveBackend [crates/dirigent_archivist/src/backend/traits.rs:33] methods=21 +['trash_session']
|
||||
[EGH] JsonlBackend defines 20/21 touched:trash_session
|
||||
[G ] MockBackend type not indexed (external/gated) impl-in: crates/dirigent_archivist/src/backend/mock.rs cfg-test new method(s) ['trash_session'] found by text in impl file (orbit can't see it)
|
||||
```
|
||||
`MockBackend` lives in a file gated by `#![cfg(any(test, feature = "test-utils"))]`. `git show 300f4319:…/mock.rs` confirms `async fn trash_session` at line 369. `20/21` means `as_search`, a default method, is not overridden. That is expected, not a finding. [V]
|
||||
|
||||
**Measured.** Over every in-repo trait at tip, grep impls were taken as ground truth (`--all-traits`):
|
||||
|
||||
| repo @ tip | grep impls | found by E or H | only grep: gated | only grep: type not indexed | only grep: cross-file/other | H false positives | E found, grep regex missed |
|
||||
|---|---|---|---|---|---|---|---|
|
||||
| dirigent @ 5de6c726 (30 traits) | 67 | 41 (61 %) | 20 | 5 | 1 | 8 (facades such as `Archivist`, `ServerRuntime`) | 0 |
|
||||
| spacecraft @ bfbbcd96 (18 traits) | 42 | 33 (79 %) | 1 | 4 | 4 | 1 | 1 (multi-line impl header) |
|
||||
|
||||
- Diff-scoped runs caught the one new trait in sc-replay (`ReplayHarnessAssertions`). Its only impl sits in another file (`[G]`), so EXTENDS has no edge; the text check found it.
|
||||
- sc-unify removed the trait `AbilityLike`. The recipe prints the grep that checks for leftover impls. [V]
|
||||
|
||||
**Blind spots:**
|
||||
- External traits (`Plugin`, `Serialize`, …) are not nodes, so there is no trait to anchor on.
|
||||
- Blanket impls.
|
||||
- `impl` on external types (`impl Ext for App`).
|
||||
- The H heuristic cannot tell a delegating facade from an implementor. Confirm H-only rows by reading.
|
||||
- The G regex misses multi-line `impl` headers.
|
||||
|
||||
**Output size:** under 1 KB per touched trait. [V]
|
||||
|
||||
## 6. Recipe 4: Module and crate boundary guards
|
||||
|
||||
**Answers:**
|
||||
- new crate→crate import edges (tip minus base, whole repo);
|
||||
- new import rows in changed files, checked against a per-repo **rules file** (allow-lists and deny patterns);
|
||||
- new intra-crate module edges (`crate::a → crate::b`);
|
||||
- two grep fallbacks, because imports alone miss edges:
|
||||
- (a) inline qualified paths (`other_crate::x::Y` with no `use`);
|
||||
- (b) **Cargo.toml dependency changes**, checked against the allow-list.
|
||||
|
||||
`--audit` checks every tip import against the rules. Use it to validate the rules themselves.
|
||||
|
||||
**Command:** `python3 $R/boundary.py out/<id>.env out/<id>.defdiff.json $R/rules/<repo>.rules [--audit] [--measure]`. Standalone SQL: `sql/crate-edges-delta.sql` (run with `--all`).
|
||||
|
||||
**Boundaries inferred from the repos:**
|
||||
- **spacecraft** (`rules/spacecraft.rules`):
|
||||
- allow-lists per crate, from AGENTS.md "Dependency Graph";
|
||||
- `warpspace_core` must not import domains/combat/hero/client (`crates/warpspace_core/CLAUDE.md`: "Dependency floor");
|
||||
- `warpspace_combat` must not import hero/client (`crates/warpspace_combat/CLAUDE.md`).
|
||||
- **dirigent** (`rules/dirigent.rules`):
|
||||
- `dirigent_acp_api` must not depend on `dirigent_core` or `dirigent_archivist` (acp_api and core CLAUDE.md);
|
||||
- `dirigent_logs` depends on no dirigent crate;
|
||||
- `dirigent_diff` has no Dioxus;
|
||||
- `dirigent_web` must not import `OpenCodeClient` (docs/architecture/rules.md);
|
||||
- `dirigent_appservice` has no workspace path deps.
|
||||
|
||||
**Sample output** (sc-replay):
|
||||
```
|
||||
new crate edges: 1
|
||||
+ warpspace_replay -> warpspace_sync_core (4 imports, e.g. crates/warpspace_replay/src/speed.rs)
|
||||
crate edges seen only as inline paths in changed files (no import row):
|
||||
~ warpspace_replay -> warpspace_protocol e.g. crates/warpspace_replay/src/harness.rs:583 [NEW: not in base imports or base inline paths]
|
||||
Cargo.toml workspace-dep changes: ['+warpspace_replay in crates/warpspace_debug/Cargo.toml', '+warpspace_sync_core in crates/warpspace_replay/Cargo.toml', '+warpspace_protocol in …']
|
||||
!! warpspace_replay gains dependency warpspace_sync_core, not in documented allow-list
|
||||
new intra-crate module edges: 3
|
||||
+ warpspace_client: interaction -> spectator
|
||||
```
|
||||
|
||||
**Findings:**
|
||||
- **Real finding, verified:** the replay feature added `warpspace_sync_core` and `warpspace_protocol` to `warpspace_replay`'s Cargo.toml. AGENTS.md still lists replay's deps as `(warpspace, domains, combat, client, engine)`. That is documentation drift a reviewer should flag. [V]
|
||||
- `warpspace_protocol` is used only through inline paths such as `warpspace_protocol::ids::UnitNetId`. The import-based edge check **missed** it; the inline-path scan caught it. [V]
|
||||
|
||||
**Measured:**
|
||||
- **Rule audits at tip:**
|
||||
- spacecraft: 0 violations with the rules as documented, and 4 once replay's documented allow-list is added. Those 4 are exactly the drift above.
|
||||
- dirigent: 7 violations, all false: `dirigent_appservice` importing its own lib by crate name. I fixed this by skipping `dst == src`, but did not re-run after the fix. [I: post-fix count expected 0]
|
||||
- **New-import detection, file level:** compared with files whose diff adds a `use` line, recall per range was 100 % (sc-replay), 85 % (sc-unify), 75 % (dg-tokens), 91 % (dg-acp), 90 % (dg-trash) and 100 % (dg-archivist). [V] The misses were:
|
||||
- `use` inside fn bodies: dg-tokens `archivist.rs` (a `use` inside a `cfg(feature)` fn), dg-move `coordinator/tests.rs`;
|
||||
- `use` lines that were only reformatted (sc-unify).
|
||||
- **Which `use` statements become import rows** (dirigent @ cd4e8f70, all 5,547 `use` lines):
|
||||
|
||||
| where the `use` line sits | recorded |
|
||||
|---|---|
|
||||
| visible code, module level | 2870/3080 (93 %; most misses are probably multi-line groups whose row line differs [I]) |
|
||||
| visible code, nested (fn body or inline mod) | 337/357 |
|
||||
| inside `#[cfg(test)]` regions | **0/621** |
|
||||
| files gated at a parent `mod`, top level | 843/897 |
|
||||
| files gated at a parent `mod`, nested | **0/350** |
|
||||
| `cfg(feature)` / other regions | 192/289 |
|
||||
|
||||
This refines phase 1's "imports are kept in gated files": that holds at top level of file-gated modules, but not inside `#[cfg(test)] mod` blocks. [V]
|
||||
|
||||
**Blind spots:**
|
||||
- Inline qualified paths. The text scan covers them only in changed files.
|
||||
- `super::`/relative paths are attributed to the file's own crate.
|
||||
- Re-exports are not followed, so a dependency through a re-exporting crate shows as an edge to that crate.
|
||||
- The compiler already enforces the Cargo graph. The guard is valuable for **documented** rules beyond Cargo (doc drift, intra-crate layering, "web must not use OpenCodeClient") and for review awareness ("first import from X to Y").
|
||||
|
||||
**Output size:** 1–2 KB. [V]
|
||||
|
||||
## 7. Recipe 5: Blindness preflight
|
||||
|
||||
**Answers:** before any orbit answer is trusted, what can orbit not see **for this diff**?
|
||||
1. **Index freshness:** manifest sha equals worktree HEAD equals the requested sha, `status=indexed`, the tree is clean, and `skipped_files`/`errored_files` are 0. A dirty tree matters because orbit indexes the working tree (§2).
|
||||
2. **Changed files with no non-macro definitions,** with size and a reason. The script walks up the `mod` chain to the gating `#[cfg]`. `macro_rules!` are indexed even in unreachable files, so they are excluded from the count.
|
||||
3. **Share of changed lines** in cfg-test, cfg-feature, other cfg regions, or gated files.
|
||||
4. **Cargo features of the touched crates.** Orbit evaluates all features off, default features included.
|
||||
5. **Added lines with constructs CALLS misses:** `add_systems`/observers, `rsx!`, `dyn`, and macro arguments.
|
||||
|
||||
**Command:** `python3 $R/preflight.py out/<id>.env out/<id>.defdiff.json`. Standalone SQL: `sql/blind-files.sql`.
|
||||
|
||||
**Sample output** (dg-acp, excerpt):
|
||||
```
|
||||
[OK] index tip: manifest=5de6c726 head=5de6c726 dirty=0 skipped=0 errored=0
|
||||
[PARTIAL] 15/58 changed .rs files have NO definitions in orbit (read/grep them):
|
||||
crates/dirigent_core/src/connectors/acp/connector.rs (238 KB) — gated at crates/dirigent_core/src/lib.rs:75 by cfg(feature = "server")
|
||||
crates/dirigent_core/src/runtime/mod.rs (176 KB) — gated at crates/dirigent_core/src/lib.rs:58 by cfg(feature = "server")
|
||||
crates/dirigent_protocol/src/adapters/opencode.rs (57 KB) — gated at crates/dirigent_protocol/src/adapters/mod.rs:5 by cfg(feature = "adapters")
|
||||
[PARTIAL] 4539/6351 changed lines (71%) are in code orbit cannot see; unattributed: cfg-test=3176, file-has-no-defs=1363, import=25, …
|
||||
[INFO] added lines with CALLS-blind constructs: assert!/matches!/vec! args (missed calls): 302
|
||||
```
|
||||
|
||||
**Measured** [V]:
|
||||
|
||||
| range | changed .rs files with no defs | changed lines orbit cannot see |
|
||||
|---|---|---|
|
||||
| sc-unify | 2/39 | 111/1725 = 6 % |
|
||||
| dg-archivist | 3/32 | 1192/5203 = 23 % |
|
||||
| dg-trash | 4/17 | 290/1273 = 23 % |
|
||||
| sc-replay | 35/72 (all `warpspace_testing/src/tests/*`) | 733/2606 = 28 % |
|
||||
| dg-tokens | 3/17 | 373/1075 = 35 % |
|
||||
| dg-move | 3/8 | 191/466 = 41 % |
|
||||
| dg-acp | 15/58 | 4539/6351 = **71 %** |
|
||||
|
||||
- **Reason lookup:** the reason given for each blind file matched the actual gate in every case I checked. Examples:
|
||||
- `connector.rs` → `lib.rs:75 #[cfg(feature = "server")] pub mod connectors;`
|
||||
- `dirigent_web/src/server.rs` → `main.rs:27`
|
||||
- The cfg-region scanner is text-based (brace matching). It handled struct-field-level `#[cfg]`, multi-line fn signatures and `#![cfg]` file attributes after two fixes, and gives the same categories as the definition-based check. [V]
|
||||
|
||||
**Output size:** 1.3–2.8 KB. [V]
|
||||
|
||||
## 8. Recipe 6: Compact context for the reviewer
|
||||
|
||||
**Answers:** the shape of the touched area as one line per type or callable, marked with change ops, within a byte budget. Changed items always show. Unchanged items show only for small files; otherwise they collapse to a count and a pointer (`orbit grep --path <file>`). Non-test files come first.
|
||||
|
||||
**Command:** `python3 $R/context.py out/<id>.env out/<id>.defdiff.json [--budget 8000]`
|
||||
|
||||
```
|
||||
## crates/dirigent_archivist/src/coordinator/meta.rs (4 changed / 18 items)
|
||||
~ pub async fn cleanup_empty_sessions(&self, archive: Option<String>) -> Result<(usize, usize)> L456
|
||||
+ pub async fn storage_usage(&self) -> Vec<ArchiveStorageUsage> L583
|
||||
… 14 unchanged items (orbit grep --path crates/dirigent_archivist/src/coordinator/meta.rs)
|
||||
## crates/dirigent_archivist/src/backend/traits.rs (2 changed / 54 items)
|
||||
+ async fn trash_session(&self, scroll_id: Uuid, trash_dir: &TrashDir) -> Result<TrashOutcome>; L117
|
||||
```
|
||||
|
||||
**Measured and compared** [V]:
|
||||
- `context.py` produced 5.3 KB (dg-trash), 8.3 KB (dg-tokens), 9.9 KB (sc-replay) and 10.5 KB (dg-acp) at budget 8000.
|
||||
- `orbit repo-map api` over the **directories** touched by dg-trash is **206 KB even after squeezing table padding**. The raw output is about 3× larger.
|
||||
- `repo-map api <file.rs>` returns `(no matching definitions under <file>/)`. It appends `/` and accepts directory prefixes only; use `orbit grep --path <file>` for a single-file outline. [V]
|
||||
- **Signature quality in `repo-map api`** (754 entries in `dirigent_archivist/src`, 367 in `dirigent_protocol/src`):
|
||||
- full signature: 71 % and 92 %;
|
||||
- truncated at `(`: 18 % and 3 %;
|
||||
- name only: 9 % and 4 %;
|
||||
- **garbage**: 2 % and 0 %. These are `let …` lines shown as signatures, e.g. `Function let base = format!("{stamp}-{suffix}"); [L438]`.
|
||||
|
||||
`context.py` re-reads declarations from source and is not affected.
|
||||
- `orbit repo-map class <Trait>` is a good one-off for a touched trait: 21 methods with signatures. Two multi-line signatures showed only the name (`trash_session`, `replace_session_messages`).
|
||||
|
||||
**Output size:** 5–10.5 KB at the default budget (about 1.3–2.6k tokens). [V]
|
||||
|
||||
---
|
||||
|
||||
## 9. End-to-end "orbit review pass"
|
||||
|
||||
This is written for Spire's `search` agent or an ad-hoc Opus reviewer. One command runs steps 1–7: `review-pass.sh <repo> <base> <tip> <name> [rules]`. It was tested at 30 s and 16 KB for dg-tokens.
|
||||
|
||||
1. **Pick the range.** For a branch, use `base = git merge-base main <branch>` and `tip = <branch>`. For an uncommitted Athanor diff, use tip = the working tree: index the checkout into an isolated DB, noting the dirty flag. For a merged PR, use `<merge>^1..<merge>`.
|
||||
2. **Set up:** `review-setup.sh`. **Stop condition:** `skipped_files > 0` or `errored_files > 0` → re-index. If it persists, list the skipped files as blind.
|
||||
3. **Preflight (recipe 5).** Decide the trust level:
|
||||
- blind share under about 25 % → orbit-led review with grep on the listed gaps;
|
||||
- 25–60 % → orbit for the visible part only; read the blind files directly;
|
||||
- over 60 % (dg-acp) → **stop using orbit for this review** except for recipes 1 and 4. Say so in the report.
|
||||
4. **Changed definitions (recipe 1).** Compare against the plan: is every planned item present, and is anything present that was not planned? Read every `S` row, every removed row, and every enum or struct with member changes.
|
||||
5. **Blast radius (recipe 2)** for `S`/`-`/member-changed rows. For each OUTSIDE-diff dependent, ask whether it needed a change it didn't get. Then run the grep for each gap class the preflight flagged:
|
||||
- **added enum variant** → grep every `match` on it;
|
||||
- **changed Bevy system signature** → check it is still registered;
|
||||
- **removed item** → confirm the old path is re-exported or gone.
|
||||
6. **Trait coverage (recipe 3)** for touched traits. Every implementor marked `!!` or `[G]` gets a read.
|
||||
7. **Boundary guard (recipe 4)** with the repo's rules file. Also compare any new crate edge or Cargo.toml dependency against the documented graph.
|
||||
8. **Context (recipe 6)** goes into the reviewer brief together with the plan.
|
||||
9. **Report:**
|
||||
- **Fact** = something orbit states and grep or a file read confirmed. Examples: "`X` signature changed", "implementor `Y` does not define new method `m` (checked in `file:line`)".
|
||||
- **Hypothesis** = any orbit count or absence ("no other callers", "no implementors", "no boundary crossing") that has not been grep-closed. Label it "lower bound (orbit)" and name the gap classes that apply.
|
||||
- **Blind** = the preflight's blind files and share, stated once at the top of the review.
|
||||
- Log one feedback record per orbit result that changed a decision, and one per result that proved wrong (§10).
|
||||
10. **Teardown:** `review-teardown.sh <repo> <name>`. Check with `git worktree list` and `pgrep -a orbit`.
|
||||
|
||||
**Stop conditions** (any one stops the pass):
|
||||
- the preflight blind share is above 60 %;
|
||||
- the index is not fresh or not clean;
|
||||
- the plan's key items are all in blind files;
|
||||
- the question is behavioural (runtime ordering, Bevy schedule, async). Orbit shows structure, not runtime.
|
||||
|
||||
## 10. Feedback record format (for the prototype skill)
|
||||
|
||||
**Format:** append-only **JSONL**, one line per orbit query whose result was used, or should have been used, in a decision. It must be cheap to write: one shell call, and the helper fills in the context.
|
||||
|
||||
```json
|
||||
{"ts":"2026-09-29T18:42:48+02:00","repo":"dirigent","commit":"d3f99fb7","dirty":false,"orbit":"0.133.0",
|
||||
"agent":"search","recipe":"blast","purpose":"callers of format_token_title","cmd":"blast.py dg-tokens",
|
||||
"rows":0,"verdict":"grep-needed","gap":"rsx","note":"both call sites inside rsx! block; git grep found them"}
|
||||
```
|
||||
|
||||
| field | required | meaning |
|
||||
|---|---|---|
|
||||
| ts, repo, commit, dirty, orbit | auto | filled in by the helper. `repo` is resolved through `git-common-dir`, so worktrees log under the main repo name |
|
||||
| agent | env `ORBIT_AGENT` | `search`, `reviewer`, `researcher`, … |
|
||||
| recipe | yes | `defdiff`, `blast`, `traitcov`, `boundary`, `preflight`, `context`, `adhoc-sql`, `grep`, `repo-map`, `context-cmd` |
|
||||
| purpose | yes | the question in a few words |
|
||||
| cmd | yes | the orbit command or script call |
|
||||
| rows | yes | result size (rows, or items reported) |
|
||||
| verdict | yes | `useful`, `wrong`, `incomplete`, `grep-needed`, `noise`, `unused` |
|
||||
| gap | optional | `cfg-test`, `cfg-feature`, `gated-file`, `dyn`, `fn-ref`, `macro`, `rsx`, `cross-file-impl`, `external-trait`, `inline-path`, `pattern-match`, `stale-index`, `sig-truncated`, `other` |
|
||||
| note | optional | one sentence. Say what grep or reading found that orbit didn't, or vice versa |
|
||||
|
||||
**Helper:** tested; it appended exactly the record above.
|
||||
```
|
||||
ORBIT_AGENT=search ~/.cache/orbit-probe/recipes/fb.sh blast grep-needed "callers of format_token_title" "blast.py dg-tokens" 0 "both call sites inside rsx! block; git grep found them" rsx
|
||||
```
|
||||
|
||||
**Where it lives** (options):
|
||||
1. **Default: `~/.local/share/orbit-skill/feedback.jsonl`.** Central, outside every repo, and survives cache cleaning. One file keeps mining across spacecraft and dirigent to a single `jq` call. `ORBIT_FEEDBACK` overrides it.
|
||||
2. **Per repo: `<repo>/local/orbit-feedback.jsonl`.** `local/` is git-ignored in both spacecraft (`.gitignore:22 /local/**`) and dirigent (`local/.gitignore: *`). It is visible to the user in the project. Worktrees have no `local/`, so the helper would have to resolve to the main checkout.
|
||||
3. **Tracked:** raw logs are not recommended. The mined **summaries** belong in `reliquary/docs/workpad/research/…/` (for example a later "phase-4 field feedback" report).
|
||||
|
||||
**Mining** [I: not built]. Example: `jq -s 'group_by(.recipe, .verdict) | map({recipe: .[0].recipe, verdict: .[0].verdict, n: length})' feedback.jsonl`, and `group_by(.gap)` for the gap histogram.
|
||||
|
||||
## 11. Questions only real usage can answer (the feedback log should capture these)
|
||||
|
||||
1. **Does the pass change review outcomes?** How often does an OUTSIDE-diff dependent (recipe 2) or an `[G]`/`!!` implementor (recipe 3) lead to a real finding, compared with noise? (`verdict=useful` vs `noise` per recipe)
|
||||
2. **Blast radius on compiled code:** since the tip compiles, how often does "caller outside the diff" matter at all? When it does, is it cfg-gated code the compiler never built? Real usage can record builds the probe was not allowed to run.
|
||||
3. **Which gap classes dominate in day-to-day diffs,** as opposed to these 7 historical ranges? In particular: Bevy fn refs in spacecraft, `rsx!` in dirigent_web, `server` gating in dirigent_core. (`gap` histogram)
|
||||
4. **Where to set the preflight thresholds:** 25 % and 60 % are guesses. Record the blind share and whether the review still found orbit useful.
|
||||
5. **Uncommitted review:** does indexing the live checkout (dirty working tree) into an isolated DB work reliably while other sessions edit the same checkout? Are there races with half-written files? (`dirty=true` records)
|
||||
6. **Is the boundary rules file worth maintaining?** How often does it catch drift, compared with how often it needs editing? (`recipe=boundary`)
|
||||
7. **Token budget:** is about 16 KB of pass output the right size for an Opus reviewer? Which sections do reviewers actually quote? (`verdict=unused` on sections)
|
||||
8. **Agent fit:** can Spire's Sonnet `search` agent run the pass and relay only the facts and hypotheses, or does the reviewer need the raw output? (`agent` field)
|
||||
9. **Moves and renames:** `>` is untested. Do real refactors produce same-fqn moves, or mostly cross-crate `-`/`+` pairs?
|
||||
10. **Feature-on indexing:** if orbit ever gains a features flag, how much of the blind share goes away? Until then, is a text-only fallback good enough for `server`-gated dirigent code?
|
||||
11. **Staleness in the wild:** how often do reviewers hit a stale or dirty index in practice? (`gap=stale-index`)
|
||||
|
||||
## 12. Corrections and additions to phase 1
|
||||
|
||||
- **Orbit indexes the working tree** (uncommitted edits and untracked non-ignored files), and the manifest records HEAD's sha. Phase 1's "uncommitted edits are not visible [I]" is wrong. [V]
|
||||
- **Imports inside `#[cfg(test)] mod` blocks are not recorded** (0/621). Neither are nested `use` statements in gated files (0/350). Phase 1's "imports kept in gated files" holds only for top-level `use` in files gated at the parent `mod`. [V]
|
||||
- **Inline qualified paths are not import rows.** Crate-edge matrices from `gl_imported_symbol` undercount. [V]
|
||||
- **`repo-map api` needs a directory prefix.** A file path returns empty silently. About 2 % of its "signatures" are `let` statements. [V]
|
||||
- **No CALLS edges target `Constant`/`Static`.** Constant reads are not edges. CALLS targets seen at tip: Function, Struct, Variant, Method, Field, AssociatedFunction, Macro, plus Python kinds. [V]
|
||||
|
||||
## 13. Scripts and files (`~/.cache/orbit-probe/recipes/`)
|
||||
|
||||
| file | recipe | purpose |
|
||||
|---|---|---|
|
||||
| `review-setup.sh` | setup | detached worktrees for base and tip, index both with `-t 4 --db <name>.duckdb`, assert `skipped_files==0`, print the env file |
|
||||
| `review-teardown.sh` | cleanup | `git worktree remove` for both, `prune`, delete the DB |
|
||||
| `review-pass.sh` | end-to-end | setup plus recipes 1–6, prints in reading order |
|
||||
| `ob.py`, `regions.py` | lib | `orbit sql` wrapper (fails on "not indexed" fallback); cfg-region scanner |
|
||||
| `defdiff.py` | 1 | changed definitions, signature deltas, unattributed-line tally, JSON for the other recipes |
|
||||
| `gt_items.py` | 1 (eval) | regex ground truth for added and removed items |
|
||||
| `blast.py` | 2 | dependents in/outside the diff, re-export check for removed items, `--measure` grep ground truth |
|
||||
| `traitcov.py` | 3 | E/G/H implementors, new-method coverage, text fallback |
|
||||
| `boundary.py`, `rules/{spacecraft,dirigent}.rules` | 4 | crate and module edge delta, rules, inline-path and Cargo.toml fallbacks |
|
||||
| `preflight.py` | 5 | blindness preflight |
|
||||
| `context.py` | 6 | budgeted API brief |
|
||||
| `fb.sh` | feedback | JSONL append helper |
|
||||
| `sql/hunk-to-defs.sql`, `dependents.sql`, `importers.sql`, `implementors.sql`, `crate-edges-delta.sql`, `blind-files.sql` | 1, 2, 2, 3, 4, 5 | standalone SQL, each tested against a real DB |
|
||||
|
||||
Phase-1 scripts (`calls.py`, `ext.py`, `defdiff.sql`, …) remain in `~/.cache/orbit-probe/`.
|
||||
|
||||
## 14. Hygiene report
|
||||
|
||||
- **Worktrees:** 16 detached worktrees were created under `~/.cache/orbit-probe/worktrees/`. All were removed with `git worktree remove` plus `git worktree prune`. At the end, `git worktree list` shows:
|
||||
- spacecraft: `main` and the user's `.worktrees/strategy-mvp`;
|
||||
- dirigent: the main checkout only.
|
||||
- **Isolated DBs:** eight per-review DBs (`*.duckdb`) and one throwaway dirty-tree DB were deleted.
|
||||
- **Shared DB:** `~/.gitlab/orbit/graph.duckdb` is unchanged: same size (72,626,176 bytes), same mtime (17:23:09) and same sha256 `c9784dbe…` before and after. The `wt-spacecraft` entry from phase 1 is still there. I did not touch it.
|
||||
- **Scratch worktree edits:** the dirty-tree test edited only my own scratch worktree. It was reverted (`git checkout -- <file>` inside that worktree, and removal of the untracked file) before the worktree was removed. No repository refs or files were modified.
|
||||
- **Processes:** no compilation was run. `pgrep -a orbit` found no orbit process at the end.
|
||||
- **Scratch size:** `~/.cache/orbit-probe/` is 1.2 MB (scripts plus text outputs in `out/`). Nothing was written to `/tmp`.
|
||||
Reference in New Issue
Block a user