Skip to content

Commit 0bb61d0

Browse files
couragehongcouragehong
committed
docs(v04): add spec parity audit for runed-grpc
13 direct spec-parity items (5 RPCs, batch split, retry, info cache, slog breadcrumb, resp count guard, status enum mapping). 3 acceptable divergences (raw stub vs runed/client wrapper, no Health retry, no Shutdown call). 4 open follow-up gaps (vector_dim verification, Health-aware classification, typed error sentinels, socket path resolver). No Python equivalent — D30 designates runed as a Go-native sibling; audit baseline is docs/v04/spec/components/embedder.md and the runed proto contract.
1 parent ecc6740 commit 0bb61d0

1 file changed

Lines changed: 134 additions & 0 deletions

File tree

‎docs/v04/notes/audit-runed-grpc.md‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# Runed-grpc Spec Parity Audit (Task 3)
2+
3+
> Run on `couragehong/feat/runed-grpc` HEAD `309e76d`.
4+
> Spec: `docs/v04/spec/components/embedder.md`.
5+
> No Python equivalent — D30 designates runed as a Go-native sibling daemon
6+
> replacing the in-process Python embedding pipeline. Audit baseline is the
7+
> spec doc + the runed daemon's actual proto contract
8+
> (`runed/proto/runed/v1/runed.proto`).
9+
10+
## Verdict
11+
12+
**Pass on the core RPC surface, two spec-promised behaviors are missing**:
13+
(1) Info.vector_dim verification on Embed responses, (2) Health-aware
14+
classification of first-failure (LOADING vs DEGRADED). Both are explicitly
15+
deferred in the embedder.md spec ("classify first failure via Health"
16+
described but never marked required for MVP). Documenting + queueing.
17+
18+
## Direct spec parity (✅)
19+
20+
| # | Spec section | Behavior | Where |
21+
|---|---|---|---|
22+
| 1 | §RPC 요약 | `Embed(text) → vector` | client.go:96-103 |
23+
| 2 | §RPC 요약 | `EmbedBatch(texts) → embeddings` | client.go:106-130 |
24+
| 3 | §RPC 요약 | `Info() → daemon_version, model_identity, vector_dim, max_text_length, max_batch_size` | info_cache.go |
25+
| 4 | §RPC 요약 | `Health() → status, uptime, total_requests` | client.go:152-164 |
26+
| 5 | §RPC 요약 | Shutdown intentionally NOT used (rune-mcp does not own daemon lifecycle) | (omitted) |
27+
| 6 | §Dial | `unix://`+sockPath, insecure creds (UDS = same machine) | client.go:71-86 |
28+
| 7 | §Retry 정책 (D7) | Backoff `[0, 500ms, 2s]` × 3 attempts | retry.go (#95) + client.go uses |
29+
| 8 | §Retry 정책 | Retryable: Unavailable / DeadlineExceeded / ResourceExhausted | retry.go:retryable |
30+
| 9 | §EmbedBatch with split | Split when len > MaxBatchSize, preserve order | client.go:106-130 |
31+
| 10 | §Info 캐시 | `sync.Once` ensures single Info RPC | info_cache.go:Get |
32+
| 11 | §Info 캐시 + D30 | `slog.Info "embedder info loaded"` w/ model_identity / vector_dim / max_batch_size | info_cache.go |
33+
| 12 | §EmbedBatch resp count guard | `len(resp.Embeddings) != len(texts)` → error | client.go:144-147 |
34+
| 13 | §Health 활용 status mapping | `STATUS_OK / LOADING / DEGRADED / SHUTTING_DOWN / UNSPECIFIED` enum→string | client.go:statusName |
35+
36+
## Acceptable divergences (⚠️)
37+
38+
1. **Hybrid choice — raw stub instead of `runed/client` Plan A wrapper**.
39+
Spec: "rune-mcp는 [runed/client] 라이브러리를 inner transport로 두고
40+
정책 layer만 자체 작성하는 hybrid 채택 가능." Implementation chose direct
41+
`runedv1.RunedServiceClient` because the wrapper exposes
42+
`Connect/Embed/EmbedBatch/Info/Close` but **NOT** `Health` (we need Health
43+
for the `rune_diagnostics` MCP tool). Documented in PR body. Net effect: a
44+
slightly larger surface area in our adapter, but no functional gap.
45+
46+
2. **Health is NOT auto-retried**. Spec §Retry 정책 lists Health among the
47+
retryable surface but D8 ("Boot does NOT poll Health — first embed call
48+
drives") implies Health is for diagnostic surfaces (vault_status,
49+
diagnostics) where transient-retry is misleading. We surface the raw error
50+
instead. Aligns with how `LifecycleService.Diagnostics` consumes it.
51+
52+
3. **`Close` does not call runed `Shutdown` RPC**. Spec §RPC 요약 explicitly
53+
"rune-mcp does not call Shutdown." Confirmed — we just close the gRPC
54+
conn.
55+
56+
## Open gaps (⚠️ should follow up)
57+
58+
### 1. **Info.vector_dim mismatch detection** (spec §불변 계약)
59+
60+
Spec: "**dim**: Qwen3-Embedding-0.6B 기준 1024. `Info.vector_dim`으로 확인 후
61+
불일치면 에러." Not implemented — `embedBatchOnce` does not verify
62+
`len(e.Vector) == c.info.Snapshot().VectorDim`. The spec's earlier prose code
63+
sample (embedder.md §EmbedBatch with split) shows this guard:
64+
65+
```go
66+
if len(e.Vector) != c.infoCache.Snapshot().VectorDim {
67+
return nil, fmt.Errorf("embedder: vector dim mismatch at index %d", i)
68+
}
69+
```
70+
71+
**Resolution**: Add the check inside `embedBatchOnce` (and a similar one
72+
inside `EmbedSingle`). Low-risk add, ~5 LoC. Track as runed follow-up.
73+
74+
### 2. **Health-aware first-failure classification** (spec §Health 활용)
75+
76+
Spec: "첫 embed 호출 실패 시 `Health` 조회로 분류:
77+
- LOADING → 잠시 후 재시도 (wait-and-retry 대기)
78+
- DEGRADED → 경고 로그 + 상위 EmbedderDegradedError 전파
79+
- SHUTTING_DOWN → 즉시 실패 + 상위 EmbedderUnavailableError"
80+
81+
Not implemented — we just bubble up the gRPC error. The retry helper does
82+
N=3 attempts with code-based retryable check, but doesn't probe Health
83+
between failures.
84+
85+
**Resolution**: This is a separate retry strategy refinement; a healthy MVP
86+
can ship with code-based retryable alone (LOADING typically surfaces as
87+
Unavailable + retryable=true → covered). Mark optional. Track as runed
88+
follow-up.
89+
90+
### 3. **Error mapping to typed adapter errors** (spec §에러 매핑 table)
91+
92+
Spec lists 5 typed error sentinels:
93+
- `EmbedderInvalidInputError` (gRPC InvalidArgument)
94+
- `EmbedderBusyError` (ResourceExhausted)
95+
- `EmbedderUnavailableError` (Unavailable)
96+
- `EmbedderTimeoutError` (DeadlineExceeded)
97+
- `EmbedderError(wrap)` (other)
98+
99+
Not implemented — `errors.go` doesn't exist in this adapter. Currently we
100+
return the raw gRPC status error wrapped with `fmt.Errorf`. Service layer
101+
relies on `status.FromError` to introspect the code, which works but lacks
102+
the typed-sentinel pattern that vault uses.
103+
104+
**Resolution**: Add `internal/adapters/embedder/errors.go` with sentinels +
105+
`MapGRPCError`. Symmetrical to `vault/errors.go`. Low-risk add. Track as
106+
runed follow-up.
107+
108+
### 4. **Socket path resolution priority is caller's responsibility**
109+
110+
Comment at the top of `client.go` documents:
111+
> 1. env RUNE_EMBEDDER_SOCKET
112+
> 2. config.embedder.socket_path
113+
> 3. default ~/.runed/embedding.sock
114+
115+
But `New(sockPath string)` takes the resolved path as input — caller must
116+
implement the priority chain. Not implemented anywhere yet (no callsite
117+
exists). **Resolution**: Add to `cmd/rune-mcp/main.go` `buildDeps` or in a
118+
new `internal/adapters/embedder/socket.go` helper, alongside the boot loop
119+
work.
120+
121+
## Test coverage status
122+
123+
`go test ./internal/adapters/embedder/` reports `[no test files]`. Mock
124+
`RunedServiceClient` + retry / split / cache tests are queued as Task #7.
125+
126+
## Cross-check: runed daemon contract compatibility
127+
128+
- runed `go.mod` requires `go 1.26.2`; rune `go.mod` is `go 1.25.9`. Local
129+
`replace ../runed` bypasses the toolchain check. **Open**: bump rune to
130+
1.26 before publishing (also needed for runed publish).
131+
- runed has `model_identity` placeholder behavior — slog breadcrumb captures
132+
whatever value it emits; we do not validate the format.
133+
- runed gen/ is `.gitignored` in runed; first-time setup requires
134+
`cd ../runed && buf generate` (or `make proto`). Document in onboarding.

0 commit comments

Comments
 (0)