Skip to content

Commit a9a084b

Browse files
AlexMikhalevAlexMikhalev
committed
Merge terraphim/task/new-model-support-v2 using published terraphim crates
Bring in the remote branch's pi_terraphim_router rewrite (KgRouter pattern using terraphim_automata::find_matches and parse_markdown_directives_dir), the readiness-aware fallback selection, the embedded planning / implementation / review tier taxonomy, and the corresponding unit tests. The remote's Cargo.toml still pointed at ../terraphim-ai/ path deps for terraphim_router / terraphim_types / terraphim_automata; that sibling checkout does not exist on this machine and the local terraphim-ai_main fork is a divergent 1.2.x. The previous commit in this branch already switched to crates.io =1.20.4 for terraphim_automata and terraphim_types, which is the coordinated release that exports the MarkdownDirectives / Thesaurus / RouteDirective types and the find_matches / parse_markdown_directives_dir functions the rewrite depends on. Drop the unused terraphim_router dep entirely and narrow the terraphim-routing feature to the two crates the code actually uses. Drop the remote branch's stray vendored crates/terraphim_settings data file and .beads runtime artefacts (.local_version, daemon-error, daemon.log); they were committed by mistake on the remote and are not part of the rewrite.
2 parents 3428d2a + 1893d26 commit a9a084b

18 files changed

Lines changed: 4025 additions & 1011 deletions

‎.docs/design-pi-rust-terraphim-router-execution-readiness.md‎

Lines changed: 458 additions & 0 deletions
Large diffs are not rendered by default.

‎.docs/design-pi-rust-terraphim-router-no-match-fallback-evidence.md‎

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
# Implementation Plan: pi_terraphim_router No-Match Fallback Evidence
2+
3+
**Status**: Draft
4+
**Research Doc**: `.docs/research-pi-rust-terraphim-router-no-match-fallback-evidence.md`
5+
**Author**: AI Agent
6+
**Date**: 2026-05-26
7+
**Estimated Effort**: 45-75 minutes
8+
9+
## Overview
10+
11+
### Summary
12+
13+
Close the remaining P2 verification gap by making no-match fallback evidence accurate and testable. The implementation should not alter public APIs or runtime behaviour; it should align private execution-selection tests with the behaviour they claim to prove.
14+
15+
### Approach
16+
17+
Make the fallback metadata a private, testable `ExecutionSelection`, then adjust tests so each scenario is explicit:
18+
19+
1. Keep `select_execution_route()` returning `None` for no taxonomy match.
20+
2. Add a private `no_match_fallback_selection()` helper that returns the existing Anthropic fallback metadata.
21+
3. Use that helper in `route_and_execute_with_registry()` so fallback metadata has one source of truth.
22+
4. Rename the current misleading test to describe matched primary selection when no registry route is ready.
23+
5. Add a true no-match planner test and a fallback metadata test.
24+
25+
### Scope
26+
27+
**In Scope:**
28+
29+
- Private helper for no-match fallback metadata.
30+
- Test rename for accurate behaviour description.
31+
- New test proving no-match planner returns `None`.
32+
- New test proving no-match fallback metadata.
33+
- Focused quality gates for router tests and formatting.
34+
35+
**Out of Scope:**
36+
37+
- Public API changes.
38+
- Real RPC/provider process tests.
39+
- Mocks or fake `RpcClient` implementations.
40+
- Changing fallback provider/model.
41+
- Addressing unrelated baseline clippy/UBS findings.
42+
43+
**Avoid At All Cost** (from 5/25 analysis):
44+
45+
- Adding test-only production abstractions around `RpcClient`.
46+
- Refactoring broader execution flow.
47+
- Converting no-match fallback into an error.
48+
- Loading auth/model registry inside pure route helpers.
49+
50+
## Architecture
51+
52+
### Component Diagram
53+
54+
```text
55+
RouterInput + Router + optional ModelRegistry
56+
|
57+
v
58+
select_execution_route()
59+
|-- explicit provider/model --> ExecutionSelection
60+
|-- taxonomy match ----------> ExecutionSelection
61+
'-- no taxonomy match -------> None
62+
|
63+
v
64+
no_match_fallback_selection()
65+
|
66+
v
67+
ExecutionSelection
68+
|
69+
v
70+
execute_selection()
71+
```
72+
73+
### Data Flow
74+
75+
```text
76+
Non-matching prompt
77+
-> select_execution_route(input, router, Some(registry))
78+
-> None
79+
-> no_match_fallback_selection()
80+
-> ExecutionSelection {
81+
provider: "anthropic",
82+
model: "claude-sonnet-4-6",
83+
capabilities: [],
84+
confidence: 0.0,
85+
reason: "fallback: no kg route matched",
86+
fallback_used: true
87+
}
88+
-> execute_selection(input, selection)
89+
```
90+
91+
### Key Design Decisions
92+
93+
| Decision | Rationale | Alternatives Rejected |
94+
|----------|-----------|----------------------|
95+
| Keep `select_execution_route()` returning `None` for no match | Preserves separation between route planning and execution fallback policy. | Making planner always return fallback selection. |
96+
| Add `no_match_fallback_selection()` | Makes fallback metadata testable without RPC spawn and avoids string drift. | Duplicating fallback strings in tests and execution. |
97+
| Rename the misleading test | Test names should describe the exercised behaviour. | Keeping a misleading test name and adding comments. |
98+
| Do not test public async fallback by spawning RPC | Avoids external process dependency and mocks. | Mocking `RpcClient`; requiring provider process in unit tests. |
99+
100+
### Eliminated Options (Essentialism)
101+
102+
| Option Rejected | Why Rejected | Risk of Including |
103+
|-----------------|--------------|-------------------|
104+
| Mock `RpcClient` | Project instruction says no mocks in tests. | Brittle test-only abstraction. |
105+
| Change fallback execution behaviour | Runtime behaviour is not the problem. | New regression risk. |
106+
| Add integration harness for RPC fallback | Too large for a P2 evidence gap. | Slow, environment-dependent test. |
107+
| Fix unrelated scanner baselines | Outside this review gap. | Scope creep. |
108+
109+
### Simplicity Check
110+
111+
> "Minimum code that solves the problem. Nothing speculative."
112+
113+
The simplest correct design is one private helper and two targeted tests. It proves the actual no-match condition and the exact fallback metadata without changing public API or requiring a provider process.
114+
115+
**Senior Engineer Test**: This is not overcomplicated if the helper has no parameters and simply returns the existing fallback metadata as `ExecutionSelection`.
116+
117+
**Nothing Speculative Checklist**:
118+
119+
- [x] No features the user did not request
120+
- [x] No abstractions in case we need them later
121+
- [x] No flexibility just in case
122+
- [x] No new runtime error handling
123+
- [x] No premature optimisation
124+
125+
## File Changes
126+
127+
### New Files
128+
129+
| File | Purpose |
130+
|------|---------|
131+
| None | Existing router module is sufficient. |
132+
133+
### Modified Files
134+
135+
| File | Changes |
136+
|------|---------|
137+
| `src/pi_terraphim_router/mod.rs` | Add private fallback metadata helper, update registry-aware fallback branch, rename/split tests. |
138+
| `.docs/research-pi-rust-terraphim-router-no-match-fallback-evidence.md` | Phase 1 research for this evidence gap. |
139+
| `.docs/design-pi-rust-terraphim-router-no-match-fallback-evidence.md` | This implementation plan. |
140+
141+
### Deleted Files
142+
143+
| File | Reason |
144+
|------|--------|
145+
| None | No deletion required. |
146+
147+
## API Design
148+
149+
### Public Types
150+
151+
No public type changes.
152+
153+
### Public Functions
154+
155+
No public function changes.
156+
157+
### Private Functions
158+
159+
Add:
160+
161+
```rust
162+
fn no_match_fallback_selection() -> ExecutionSelection {
163+
ExecutionSelection {
164+
provider: "anthropic".to_string(),
165+
model: "claude-sonnet-4-6".to_string(),
166+
capabilities: vec![],
167+
confidence: 0.0,
168+
reason: "fallback: no kg route matched".to_string(),
169+
fallback_used: true,
170+
}
171+
}
172+
```
173+
174+
Use in `route_and_execute_with_registry()`:
175+
176+
```rust
177+
let selection = select_execution_route(&input, &router, Some(registry))
178+
.unwrap_or_else(no_match_fallback_selection);
179+
execute_selection(input, selection).await
180+
```
181+
182+
Do not change `route_and_execute(input)` unless a separate design decision approves sharing fallback metadata there too. Optional minimal sharing is allowed only if it does not increase diff size meaningfully.
183+
184+
### Error Types
185+
186+
No error type changes.
187+
188+
## Test Strategy
189+
190+
### Unit Tests
191+
192+
| Test | Location | Purpose |
193+
|------|----------|---------|
194+
| `test_execution_selection_with_registry_uses_primary_when_none_ready` | `src/pi_terraphim_router/mod.rs` | Renamed current test; proves matched route with no ready registry still selects primary. |
195+
| `test_execution_selection_with_registry_returns_none_without_match` | `src/pi_terraphim_router/mod.rs` | Uses a non-matching prompt and proves planner returns `None`. |
196+
| `test_no_match_fallback_selection_uses_default_anthropic_route` | `src/pi_terraphim_router/mod.rs` | Proves fallback metadata without launching RPC. |
197+
198+
### Integration Tests
199+
200+
No new integration test required. The public async function still spawns RPC, so its fallback branch is covered through private metadata helper plus compile-time checks rather than external execution.
201+
202+
### Coverage Expectations
203+
204+
- No test should claim no-match fallback while using a matching prompt.
205+
- Fallback metadata should have one assertion each for provider, model, capabilities, confidence, reason, and `fallback_used`.
206+
- Existing first-ready fallback and explicit preference tests remain unchanged.
207+
208+
## Implementation Steps
209+
210+
### Step 1: Add Private Fallback Selection Helper
211+
212+
**Files:** `src/pi_terraphim_router/mod.rs`
213+
**Description:** Add `no_match_fallback_selection() -> ExecutionSelection` beside `select_execution_route()`. It must return the same fallback metadata currently hardcoded in `route_and_execute_with_registry()`.
214+
**Tests:** Add `test_no_match_fallback_selection_uses_default_anthropic_route`.
215+
**Estimated:** 15 minutes
216+
217+
### Step 2: Use Helper in Registry-Aware Execution
218+
219+
**Files:** `src/pi_terraphim_router/mod.rs`
220+
**Description:** Replace the manual no-match fallback branch in `route_and_execute_with_registry()` with `unwrap_or_else(no_match_fallback_selection)` and `execute_selection(input, selection).await`. This preserves runtime behaviour while making metadata single-source.
221+
**Tests:** Existing tests plus fallback metadata helper test.
222+
**Dependencies:** Step 1
223+
**Estimated:** 15 minutes
224+
225+
### Step 3: Fix Misleading Test Coverage
226+
227+
**Files:** `src/pi_terraphim_router/mod.rs`
228+
**Description:** Rename `test_execution_selection_with_registry_uses_no_match_fallback` to `test_execution_selection_with_registry_uses_primary_when_none_ready`. Add `test_execution_selection_with_registry_returns_none_without_match` using a prompt that does not contain any configured synonym.
229+
**Tests:** The renamed and new tests.
230+
**Dependencies:** None
231+
**Estimated:** 15 minutes
232+
233+
### Step 4: Quality Gates
234+
235+
**Files:** All changed files
236+
**Description:** Run focused and standard checks.
237+
**Tests:**
238+
239+
```bash
240+
rch exec -- cargo check --all-targets --features terraphim-routing
241+
rch exec -- cargo test --features terraphim-routing -- pi_terraphim_router
242+
cargo fmt --check
243+
```
244+
245+
Run clippy and UBS as evidence, but document known repository baseline noise if unchanged-file findings dominate:
246+
247+
```bash
248+
rch exec -- cargo clippy --all-targets --features terraphim-routing -- -D warnings
249+
ubs --only=rust src/pi_terraphim_router/mod.rs src/main.rs
250+
```
251+
252+
If committing staged Rust changes, prefer changed-line UBS gate when full-file scan is noisy:
253+
254+
```bash
255+
python3 scripts/check_ubs_staged_delta.py
256+
```
257+
258+
**Dependencies:** Steps 1-3
259+
**Estimated:** 20-30 minutes
260+
261+
## Rollback Plan
262+
263+
If issues are discovered:
264+
265+
1. Revert the no-match evidence follow-up commit.
266+
2. Keep `91b16777` intact because it added the registry-aware execution API.
267+
3. If helper extraction proves undesirable, use the minimal test-only approach: assert `select_execution_route()` returns `None` for a non-matching prompt and leave runtime code unchanged.
268+
269+
## Migration
270+
271+
No migration required. No public API changes.
272+
273+
## Dependencies
274+
275+
### New Dependencies
276+
277+
| Crate | Version | Justification |
278+
|-------|---------|---------------|
279+
| None | N/A | Existing private types are sufficient. |
280+
281+
### Dependency Updates
282+
283+
| Crate | From | To | Reason |
284+
|-------|------|----|--------|
285+
| None | N/A | N/A | No dependency changes needed. |
286+
287+
## Performance Considerations
288+
289+
### Expected Performance
290+
291+
| Metric | Target | Measurement |
292+
|--------|--------|-------------|
293+
| Route planning latency | No material change | One private helper only used on no-match branch. |
294+
| RPC execution latency | No change | Same provider/model fallback and same `execute_selection()` path. |
295+
| Memory | No material change | One short-lived `ExecutionSelection`. |
296+
297+
### Benchmarks to Add
298+
299+
No benchmark required. This is a testability/evidence fix with no meaningful hot-path change.
300+
301+
## Open Items
302+
303+
| Item | Status | Owner |
304+
|------|--------|-------|
305+
| Decide whether to also use `no_match_fallback_selection()` in compatibility `route_and_execute(input)` | Optional, pending implementation judgement | Implementer |
306+
| Decide whether baseline scanner findings need separate issue tracking | Outside scope | User/repo owner |
307+
308+
## Approval
309+
310+
- [ ] Technical review complete
311+
- [ ] Test strategy approved
312+
- [ ] Performance targets agreed
313+
- [ ] Human approval received

0 commit comments

Comments
 (0)