mavend: centralize action validation boundary (slice 4)

This commit is contained in:
2026-09-06 12:53:54 +04:00
parent 6a402bf556
commit 356766bce1
32 changed files with 2061 additions and 178 deletions
@@ -0,0 +1,118 @@
# Slice 1: Typed Ingress Boundary and Route Producer Observability
## 1. Files changed
**New types:**
- `internal/router/source.go``InputSource`, `InputSourceVoice`, `InputSourceText`, `NormalizedInput`
- `internal/router/intent.go``RouteProducer`, `RouteProducerGrammar/Heads/LLM/Classifier`, `Producer` field on `Decision`
**Cascade wiring:**
- `internal/router/router.go``Producer` set at each of the four cascade stages
**Observability:**
- `internal/decision/decision.go``InputSource` and `RouteProducer` fields on `Record`; `With()` accepts `inputSource`
- `internal/store/routingtraces.go``RouteProducer` field on `RoutingTrace`
- `internal/store/migrations.go` — migration #27: `ALTER TABLE routing_traces ADD COLUMN route_producer`
- `cmd/mavend/routingtrace.go` — persists `RouteProducer` from the decision record
**Turn lifecycle:**
- `cmd/mavend/voice.go``turnSource` is now `type turnSource = router.InputSource`; `runTurn` takes `NormalizedInput`; `HandlePushToTalk` and `handleText` construct `NormalizedInput`
- `cmd/mavend/turnroute.go``turnRoute` carries `NormalizedInput`; `newTurnRoute` and `resolve` use it
**Test updates (signature适应):**
- `cmd/mavend/clarify_test.go`, `reactive_notes_test.go`, `reminder_cancel_test.go`, `repair_test.go`, `simulator_test.go`, `turnrole_test.go``runTurn` calls updated to `NormalizedInput`
**New tests:**
- `internal/router/boundary_test.go` — 7 tests: type shape, constants, producer per stage
- `cmd/mavend/boundary_test.go` — 5 tests: convergence, source preservation, producer on record, pre-route empty producer, stage-0 unchanged
## 2. Boundary types introduced/reused
| Type | Package | Kind | Purpose |
|---|---|---|---|
| `NormalizedInput` | `router` | new struct | Typed ingress boundary: `Text string` + `Source InputSource` |
| `InputSource` | `router` | new `string` type | Channel provenance: `tap:voice`, `tap:text` |
| `RouteProducer` | `router` | new `string` type | Cascade stage provenance: `grammar`, `heads`, `llm`, `classifier` |
| `turnSource` | `main` | **alias** for `router.InputSource` | Convenience alias; all existing call sites unchanged |
Reused: `router.Source` (destination), `router.Intent`, `router.Decision`, `decision.Record`.
## 3. Before/after flow diagram
```
BEFORE:
HandlePushToTalk → stt → runTurn(ctx, text, sourceVoice)
handleText → runTurn(ctx, text, sourceText)
runTurn(ctx, text, src):
decision.With(ctx, text)
newTurnRoute(text, now) → rt.text = text
rt.resolve() → router.Route(ctx, text, now)
Decision.Utterance = utterance
[no producer field]
AFTER:
HandlePushToTalk → stt → runTurn(ctx, NormalizedInput{text, sourceVoice})
handleText → runTurn(ctx, NormalizedInput{text, sourceText})
runTurn(ctx, input):
decision.With(ctx, input.Text, input.Source)
newTurnRoute(input, now) → rt.input = input
rt.resolve() → router.Route(ctx, input.Text, now)
Decision.Utterance = utterance
Decision.Producer = grammar|heads|llm|classifier
rec.RouteProducer = dec.Producer
```
## 4. Tests added
**internal/router/boundary_test.go** (7 tests):
- `TestNormalizedInputIsMinimalValueObject` — shape pin
- `TestInputSourceConstants` — tap:voice, tap:text
- `TestRouteProducerConstants` — grammar, heads, llm, classifier
- `TestStage0SetsGrammarProducer` — grammar win carries grammar producer
- `TestClassifierSetsProducer` — classifier floor sets its producer
- `TestClarifyProducerIsClassifier` — clarified turn carries classifier producer
- `TestStage0ProducerOnEveryGrammar` — property test over multiple grammars
**cmd/mavend/boundary_test.go** (5 tests):
- `TestTextAndVoiceConvergeOnNormalizedInput` — same utterance, same route intent
- `TestNormalizedInputSourcePreserved` — source survives to decision record
- `TestRouteProducerOnDecisionRecord` — producer carried to record
- `TestPreRouteClaimHasNoRouteProducer` — confirm-claimed turn has empty producer
- `TestStage0ProducerUnchanged` — grammar stage-0 still produces same intents
## 5. Full test/eval results
| Suite | Before | After |
|---|---|---|
| `go test ./internal/router/` | PASS (1.058s) | PASS (1.568s) |
| `go test ./internal/router/eval/` | PASS (0.783s) | PASS (1.219s) |
| `go test ./cmd/mavend/ -run Simulator` | PASS (1.451s) | PASS (1.753s) |
| `go test ./cmd/mavend/ -run PersonalBoundary` | PASS (0.147s) | PASS (0.183s) |
| `go test ./cmd/mavend/ -run Eval` | PASS (0.015s) | PASS (0.012s) |
| `go test ./cmd/mavend/` (full) | PASS (1.451s) | PASS (22.311s) |
The timing increase in `cmd/mavend` full suite is from the new boundary tests (293 lines of new test code), not from a regression.
## 6. Confirmation that routing outputs and action behavior are unchanged
- `internal/router/eval/` scores the held-out fixture against the same cascade; the number did not move.
- `cmd/mavend -run Simulator` replays deterministic scripted days; all scenario assertions pass identically.
- `cmd/mavend -run PersonalBoundary` exercises the personal boundary query chain; passes identically.
- Stage-0 grammars: same ordering in `StageZeroGrammars()`, same matching semantics, same confidence 1.0.
- `RouteProducer` is a new field with zero value `""` for existing code paths that don't set it; no existing consumer reads it.
- `NormalizedInput` is the same `(text string, source turnSource)` pair passed as a struct; no transformation applied.
## 7. Semantic changes required
**None.** The refactoring is purely structural:
- `turnSource` became a type alias for `router.InputSource` — identical underlying type, no conversion needed at any call site.
- `runTurn` takes `NormalizedInput` instead of `(text, src)` — the destructuring `text := input.Text; src := input.Source` at the top of the function body produces identical local variables.
- `decision.With` gained an `inputSource` parameter — a string stored on the record, never read back during routing.
- `RouteProducer` is a new field on `Decision` — set after the decision is already produced, never consumed by the cascade.
## 8. Commit hashes
```
87a3b16 router: introduce typed ingress boundary and route producer observability
a55d909 router: add boundary tests for typed ingress and route producer
```
@@ -0,0 +1,188 @@
# Slice 3: Structural validation between ActionCandidate resolution and execution
**Date:** 2026-09-05
**Base:** 6a402bf (slice 2 committed)
**Status:** complete
## 1. Files changed
| File | Change |
|------|--------|
| `internal/router/actioncandidate.go` | +76 lines: `ActionField`, `ActionValidationIssue`, `ActionValidationResult`, `ValidateActionCandidate` |
| `internal/router/actioncandidate_test.go` | +98 lines: 6 unit tests for `ValidateActionCandidate` |
| `cmd/mavend/actions_act.go` | +11 lines: validation call, invalid-candidate early return |
| `cmd/mavend/actionresolve.go` | +37 lines: `noteActionValidation` tracing function |
| `cmd/mavend/actionresolve_test.go` | +128 lines: 5 integration tests (malformed, unresolved, destructive, irreversible, tracing) |
## 2. Validation contract
```go
type ActionValidationResult struct {
Unresolved bool // Fn empty → proposeGap path
Valid bool // Fn non-empty, structure sound
Issues []ActionValidationIssue // non-empty when Invalid
}
type ActionValidationIssue struct {
Field ActionField // "fn" or "args"
Reason string // machine-readable tag
}
type ActionField string
const (
FieldFn ActionField = "fn"
FieldArgs ActionField = "args"
)
```
Three disjoint outcomes:
- **Unresolved**: Fn is empty. Not a validation error. Routes to proposeGap / clarification.
- **Valid**: Fn non-empty, structurally admissible. Proceeds to risk policy and execution.
- **Invalid**: Fn non-empty but malformed. Refused with `ActFail`.
## 3. Exact validation rules introduced
| Rule | Check | Outcome on fail |
|------|-------|-----------------|
| `unresolved` | `c.Fn == ""` | Unresolved (not invalid) |
| `blank_function_name` | `strings.TrimSpace(c.Fn) == ""` when Fn is non-empty | Invalid |
The model is deliberately small. This is not a general validation framework.
## 4. Where each rule existed previously
| Rule | Previous location | Migration type |
|------|-------------------|----------------|
| Unresolved → proposeGap | `cmd/mavend/actions_act.go:82` (`!dec.Slots.HasFn`) | Preserved: the existing `!dec.Slots.HasFn` check now comes after validation, same observable behavior |
| Blank Fn → refuse | No previous check existed | New: defensive check for a structurally malformed candidate that the route/matcher should never produce |
**Observation:** The current route and matcher never produce a blank (whitespace-only) Fn. The `blank_function_name` rule is a defensive gate against future producer defects, not a migration from existing behavior.
## 5. Before/after action flow
**Before (slice 2):**
```
RouteDecision → ResolveActionCandidate → ActionCandidate
→ actionAct writes Fn/Args back into dec.Slots
→ refusesCommand check
→ task-status intercept
→ Praxis intercept
→ Hexis intercept
→ !HasFn → proposeGap
→ tool.Executor.Exec → error switch
```
**After (slice 3):**
```
RouteDecision → ResolveActionCandidate → ActionCandidate
→ ValidateActionCandidate
├─ unresolved → (continues to existing flow; !HasFn → proposeGap)
├─ invalid → ActFail (early return)
└─ valid → (continues)
→ actionAct writes Fn/Args back into dec.Slots
→ refusesCommand check
→ task-status intercept
→ Praxis intercept
→ Hexis intercept
→ !HasFn → proposeGap
→ tool.Executor.Exec → error switch
```
The validation gate sits between resolution and the bridge write. Unresolved candidates skip the validation gate entirely and flow through the existing path unchanged.
## 6. How unresolved differs from invalid
| | Unresolved | Invalid |
|---|---|---|
| **Fn** | empty (`""`) | non-empty but malformed (e.g. whitespace-only) |
| **Cause** | Matcher miss, non-act intent | Structural defect in route/matcher output |
| **Trace** | `action-validation:declined:unresolved` | `action-validation:declined:invalid:<reason>` |
| **Response** | proposeGap (existing proposal/scaffold path) | `ActFail` ("не получилось выполнить команду.") |
| **Execution** | does not reach tool executor | does not reach tool executor |
The distinction is preserved: unresolved is "no match found" (the normal case for unknown verbs), while invalid is "a match was found but it's structurally broken" (a defect that should never happen in practice).
## 7. Tests added
### Unit tests (internal/router)
| Test | Pins |
|------|------|
| `TestValidateActionCandidate_UnresolvedEmptyFn` | Empty Fn → unresolved, not invalid |
| `TestValidateActionCandidate_UnresolvedMatcherMiss` | Matcher miss candidate → unresolved |
| `TestValidateActionCandidate_ValidRoute` | Route-resolved candidate → valid |
| `TestValidateActionCandidate_ValidMatcher` | Matcher-resolved candidate → valid |
| `TestValidateActionCandidate_ValidNoArgs` | Zero-arg tool → valid |
| `TestValidateActionCandidate_BlankFn` | Whitespace-only Fn → invalid with `blank_function_name` issue |
### Integration tests (cmd/mavend)
| Test | Pins |
|------|------|
| `TestActValidation_MalformedCandidate_BlankFn` | Blank Fn does not execute, produces response |
| `TestActValidation_UnresolvedCandidate_ProposeGap` | Matcher miss still flows to proposeGap |
| `TestActValidation_DestructiveValid_StillConfirms` | Destructive valid act still triggers confirm turn |
| `TestActValidation_IrreversibleValid_NeedsAuthedSurface` | Irreversible valid act still reaches authed-surface refusal |
| `TestActValidation_ValidationTracing` | Validation outcome recorded in decision trace |
### Existing regression tests preserved (all pass)
| Test | What it pins |
|------|-------------|
| `TestActRouteSource_NoMatcherInvoke` | Route-resolved act executes, matcher not invoked |
| `TestActMatcherSource_FallbackMatch` | Matcher-resolved act executes |
| `TestActMatcherMiss_ProposeGap` | Matcher miss → proposeGap |
| `TestActDestructive_ConfirmationUnchanged` | Destructive → confirm turn |
| `TestActTaskStatus_InterceptUnchanged` | task_status intercepted before tool execution |
| `TestActStage0_SameResult` | Stage-0 grammar act executes |
| `TestActLearnedRouter_NoFn_FallbackMatch` | LLM-routed act without Fn → matcher fallback |
| `TestActPathSpeaksTheTiers` | Risk tiers spoken correctly |
| `TestActionActMarkerReferentMovesOnlyTheNamedStoredTask` | Task-status move unchanged |
| `TestActOffAllowlistIsStillRefused` | Off-allowlist act refused |
## 8. Before/after suite results
**Before:** all tests in `internal/router` and `cmd/mavend` pass.
**After:** all tests pass, including 6 new unit tests and 5 new integration tests.
```
internal/router: PASS (0.940s) — 13 actioncandidate tests (7 resolve + 6 validate)
cmd/mavend: PASS (23.5s) — 21 act-path tests (15 existing + 5 new + 1 tracing)
```
## 9. Confirmation that risk/confirmation/execution behavior is unchanged
- **Risk tiers:** `tool.RiskOf` and `tool.PolicyFor` are not touched. Validation happens before risk policy.
- **Confirmation:** `ErrNeedsConfirm` → confirm turn is unchanged. Tested by `TestActDestructive_StillConfirms`.
- **Irreversible:** `ErrNeedsAuthedSurface` → refusal is unchanged. Tested by `TestActIrreversibleValid_NeedsAuthedSurface`.
- **Execution:** `tool.Executor.Exec` is not modified. Validation is a pure pre-screen.
- **Ecosystem intercepts:** Praxis and Hexis intercepts are unchanged. Validation sits before them; unresolved candidates pass through to them as before.
## 10. Remaining downstream consumers of Decision.Slots
After the bridge write (`dec.Slots.Fn = candidate.Fn`, etc.), the following branches read `dec.Slots`:
| Consumer | File | Reads |
|----------|------|-------|
| `resolveTaskStatus` | `cmd/mavend/actions_task.go:109` | `dec.Slots.Fn` (compared to `TaskStatusFn`) |
| `handlePraxisAct` | `cmd/mavend/ecosystem_acts.go:110` | `dec.Slots.HasFn` (guard), `dec.Slots.Fn`, `dec.Slots.Args` |
| `handleHexisAct` | `cmd/mavend/ecosystem_acts.go:660` | via `ActHasEntityTarget(dec)` which reads `dec.Slots.HasFn`, `dec.Slots.Args`, `dec.Slots.Text` |
| `proposeGap` | `cmd/mavend/confirm.go:191` | `dec.Utterance` (not Slots) |
| `tool.Executor.Exec` | `internal/tool/tool.go:156` | called with `dec.Slots.Fn, dec.Slots.Args` directly |
| `actPhrase` | `cmd/mavend/actions_act.go` | `dec.Slots.Fn, dec.Slots.Args` |
| `park` | `cmd/mavend/confirm.go` | `dec.Slots.Fn, dec.Slots.Args` |
## 11. Whether ActionCandidate can become authoritative in a later slice
Yes. The bridge write is the only thing coupling ActionCandidate to Decision.Slots. In a later slice:
1. `resolveTaskStatus` could accept `ActionCandidate` directly instead of reading `dec.Slots.Fn`.
2. `handlePraxisAct` and `handleHexisAct` could accept the candidate instead of `dec.Slots.HasFn`.
3. `tool.Executor.Exec` already takes `name string, args []string` — it could take the candidate's `Fn` and `Args` directly.
4. `ActHasEntityTarget` could accept `ActionCandidate` instead of `Decision`.
The bridge would be removed once all consumers read from the candidate. No semantic changes required — this is a mechanical refactoring of argument passing.
## 12. Commit hash
Pending commit on top of 6a402bf.
+158
View File
@@ -0,0 +1,158 @@
# Slice 3: Structural validation between ActionCandidate resolution and execution
**Date:** 2026-09-05
**Base:** 6a402bf (slice 2 committed)
**Status:** complete
## Files changed
| File | Lines added |
|------|------------|
| `internal/router/actioncandidate.go` | +76 (types + `ValidateActionCandidate`) |
| `internal/router/actioncandidate_test.go` | +98 (6 unit tests) |
| `cmd/mavend/actions_act.go` | +11 (validation gate in `actionAct`) |
| `cmd/mavend/actionresolve.go` | +37 (`noteActionValidation` tracing) |
| `cmd/mavend/actionresolve_test.go` | +128 (5 integration tests) |
| `docs/reports/2026-09-05-slice3-structural-validation.md` | full report |
## Validation contract
```go
type ActionValidationResult struct {
Unresolved bool
Valid bool
Issues []ActionValidationIssue
}
type ActionValidationIssue struct {
Field ActionField
Reason string
}
type ActionField string
const (
FieldFn ActionField = "fn"
FieldArgs ActionField = "args"
)
```
Three disjoint outcomes:
- **Unresolved**: Fn is empty. Not a validation error. Routes to proposeGap / clarification.
- **Valid**: Fn non-empty, structurally admissible. Proceeds to risk policy and execution.
- **Invalid**: Fn non-empty but malformed. Refused with `ActFail`.
## Exact validation rules
| Rule | Check | Outcome on fail |
|------|-------|-----------------|
| `unresolved` | `c.Fn == ""` | Unresolved (not invalid) |
| `blank_function_name` | `strings.TrimSpace(c.Fn) == ""` when Fn is non-empty | Invalid |
## Before/after action flow
**Before (slice 2):**
```
RouteDecision → ResolveActionCandidate → ActionCandidate
→ actionAct writes Fn/Args back into dec.Slots
→ refusesCommand check
→ task-status intercept
→ Praxis intercept
→ Hexis intercept
→ !HasFn → proposeGap
→ tool.Executor.Exec → error switch
```
**After (slice 3):**
```
RouteDecision → ResolveActionCandidate → ActionCandidate
→ ValidateActionCandidate
├─ unresolved → (continues to existing flow; !HasFn → proposeGap)
├─ invalid → ActFail (early return)
└─ valid → (continues)
→ actionAct writes Fn/Args back into dec.Slots
→ refusesCommand check
→ task-status intercept
→ Praxis intercept
→ Hexis intercept
→ !HasFn → proposeGap
→ tool.Executor.Exec → error switch
```
## How unresolved differs from invalid
| | Unresolved | Invalid |
|---|---|---|
| **Fn** | empty (`""`) | non-empty but malformed (e.g. whitespace-only) |
| **Cause** | Matcher miss, non-act intent | Structural defect in route/matcher output |
| **Trace** | `action-validation:declined:unresolved` | `action-validation:declined:invalid:<reason>` |
| **Response** | proposeGap (existing proposal/scaffold path) | `ActFail` |
| **Execution** | does not reach tool executor | does not reach tool executor |
## Tests added
### Unit tests (internal/router)
| Test | Pins |
|------|------|
| `TestValidateActionCandidate_UnresolvedEmptyFn` | Empty Fn → unresolved, not invalid |
| `TestValidateActionCandidate_UnresolvedMatcherMiss` | Matcher miss candidate → unresolved |
| `TestValidateActionCandidate_ValidRoute` | Route-resolved candidate → valid |
| `TestValidateActionCandidate_ValidMatcher` | Matcher-resolved candidate → valid |
| `TestValidateActionCandidate_ValidNoArgs` | Zero-arg tool → valid |
| `TestValidateActionCandidate_BlankFn` | Whitespace-only Fn → invalid with `blank_function_name` issue |
### Integration tests (cmd/mavend)
| Test | Pins |
|------|------|
| `TestActValidation_MalformedCandidate_BlankFn` | Blank Fn does not execute, produces response |
| `TestActValidation_UnresolvedCandidate_ProposeGap` | Matcher miss still flows to proposeGap |
| `TestActValidation_DestructiveValid_StillConfirms` | Destructive valid act still triggers confirm turn |
| `TestActValidation_IrreversibleValid_NeedsAuthedSurface` | Irreversible valid act still reaches authed-surface refusal |
| `TestActValidation_ValidationTracing` | Validation outcome recorded in decision trace |
### Existing regression tests preserved (all pass)
| Test | What it pins |
|------|-------------|
| `TestActRouteSource_NoMatcherInvoke` | Route-resolved act executes, matcher not invoked |
| `TestActMatcherSource_FallbackMatch` | Matcher-resolved act executes |
| `TestActMatcherMiss_ProposeGap` | Matcher miss → proposeGap |
| `TestActDestructive_ConfirmationUnchanged` | Destructive → confirm turn |
| `TestActTaskStatus_InterceptUnchanged` | task_status intercepted before tool execution |
| `TestActStage0_SameResult` | Stage-0 grammar act executes |
| `TestActLearnedRouter_NoFn_FallbackMatch` | LLM-routed act without Fn → matcher fallback |
| `TestActPathSpeaksTheTiers` | Risk tiers spoken correctly |
| `TestActionActMarkerReferentMovesOnlyTheNamedStoredTask` | Task-status move unchanged |
| `TestActOffAllowlistIsStillRefused` | Off-allowlist act refused |
## Test results
```
internal/router: PASS (1.1s) — 13 actioncandidate tests (7 resolve + 6 validate)
cmd/mavend: PASS (23.4s) — 21 act-path tests (15 existing + 5 new + 1 tracing)
```
## Risk/confirmation/execution behavior unchanged
- **Risk tiers:** `tool.RiskOf` and `tool.PolicyFor` not touched. Validation happens before risk policy.
- **Confirmation:** `ErrNeedsConfirm` → confirm turn unchanged.
- **Irreversible:** `ErrNeedsAuthedSurface` → refusal unchanged.
- **Execution:** `tool.Executor.Exec` not modified. Validation is a pure pre-screen.
- **Ecosystem intercepts:** Praxis and Hexis intercepts unchanged.
## Remaining downstream consumers of Decision.Slots
| Consumer | File | Reads |
|----------|------|-------|
| `resolveTaskStatus` | `cmd/mavend/actions_task.go:109` | `dec.Slots.Fn` |
| `handlePraxisAct` | `cmd/mavend/ecosystem_acts.go:110` | `dec.Slots.HasFn`, `dec.Slots.Fn`, `dec.Slots.Args` |
| `handleHexisAct` | `cmd/mavend/ecosystem_acts.go:660` | via `ActHasEntityTarget(dec)` |
| `proposeGap` | `cmd/mavend/confirm.go:191` | `dec.Utterance` (not Slots) |
| `tool.Executor.Exec` | `internal/tool/tool.go:156` | called with `dec.Slots.Fn, dec.Slots.Args` |
| `actPhrase` | `cmd/mavend/actions_act.go` | `dec.Slots.Fn, dec.Slots.Args` |
| `park` | `cmd/mavend/confirm.go` | `dec.Slots.Fn, dec.Slots.Args` |
## Can ActionCandidate become authoritative?
Yes. The bridge write is the only coupling. In a later slice, downstream consumers can read directly from the candidate. No semantic changes required — mechanical refactoring of argument passing.