From 99f4f9a0db1d918eff59a08e610b32c1f4279020 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 29 Aug 2026 19:43:28 +0000 Subject: [PATCH] Execute incremental analysis artifact plans --- docs/cli.md | 3 +- docs/config.md | 13 +- docs/internal/artifacts.md | 6 + docs/internal/manifest.md | 9 + docs/internal/stage-analyze.md | 50 ++- docs/operations.md | 3 +- docs/roadmap/implementation.md | 2 + internal/artifacts/catalog.go | 35 ++ internal/artifacts/catalog_test.go | 31 +- internal/stage/analyze.go | 355 +++++++++++------- .../analyze_incremental_execution_test.go | 249 ++++++++++++ internal/stage/analyze_order_test.go | 44 --- internal/stage/analyze_test.go | 24 +- 13 files changed, 613 insertions(+), 211 deletions(-) create mode 100644 internal/stage/analyze_incremental_execution_test.go delete mode 100644 internal/stage/analyze_order_test.go diff --git a/docs/cli.md b/docs/cli.md index 3b1bcbc..1e4031c 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -294,7 +294,8 @@ and precedence. Effects: -- filters analyze execution to selected configured artifacts; +- selects explicit analyze targets; required configured prerequisites may be + reused or rebuilt before them; - filters publish rules that source `narratio.artifact.`; - does not filter built-in transcript/bounds or explicitly configured `narratio.extraction.` publish sources; and diff --git a/docs/config.md b/docs/config.md index c1d9099..328df40 100644 --- a/docs/config.md +++ b/docs/config.md @@ -323,12 +323,13 @@ Narratio adds `session_id=narratio-session-` to every Scriptorium re Without `--artifacts`, analyze executes enabled configured artifacts. With an explicit `--artifacts` list, the exact named configured artifacts are the -one-invocation execution set even if their `enabled` values are false; the list -does not automatically include dependencies. Named artifacts must therefore be -configured with valid executable fields, and their configured dependencies must -already be available to analyze. This override affects analyze planning only; -publish uses the list only to filter configured -`narratio.artifact.` output rules. +one-invocation targets even if their `enabled` values are false. Analyze closes +those targets over `depends_on`: a current prerequisite is reused, while a +stale, missing, failed, or legacy prerequisite is rebuilt before its dependent. +Unrelated artifacts are not executed. Named targets and any prerequisite that +may require rebuilding must therefore have valid executable fields. This +override affects analyze planning only; publish uses the list only to filter +configured `narratio.artifact.` output rules. For each artifact input `pipeline.scriptorium.artifacts..inputs.`: diff --git a/docs/internal/artifacts.md b/docs/internal/artifacts.md index cfa0709..0682484 100644 --- a/docs/internal/artifacts.md +++ b/docs/internal/artifacts.md @@ -63,6 +63,12 @@ canonical file and a legacy aggregate analyze output are unavailable. Extraction entries are registered from configuration and become available only after compatible extraction evidence is hydrated. +During an analyze invocation, a newly validated and atomically materialized +configured output is marked available with its producer run ID, contract, +checksum, and size. Later scheduled dependents therefore observe the same +semantic identity whether their prerequisite was reused from current manifest +evidence or produced earlier in the invocation. + Current provenance values: - `generated.current_analyze_run` diff --git a/docs/internal/manifest.md b/docs/internal/manifest.md index 1c24605..d7b84d6 100644 --- a/docs/internal/manifest.md +++ b/docs/internal/manifest.md @@ -74,6 +74,15 @@ limited to current records produced by that invocation's run ID. Ordinary stage outputs cannot accompany this projection, so there is one source of artifact authority. +Successful incremental execution replaces only evaluated artifact records and +preserves valid unrelated current records. Rebuilt outputs are compared by +bytes and contract: an unchanged identity permits an unselected dependent with +the same recomputed fingerprint to remain current, while a changed identity +removes output authority from every unselected transitive dependent by marking +it stale. A partial analyze invocation can therefore succeed while unrelated +configured records remain stale. Existing canonical files never create current +records without validated execution and projection. + Analyze may return a projection together with an error. That restricted result cannot carry ordinary outputs, skip state, aggregate logs, generated configs, or metadata. The runner persists only the validated per-artifact collections, diff --git a/docs/internal/stage-analyze.md b/docs/internal/stage-analyze.md index 25c6071..80164d1 100644 --- a/docs/internal/stage-analyze.md +++ b/docs/internal/stage-analyze.md @@ -2,7 +2,8 @@ ## Purpose -Execute selected configured Scriptorium artifacts in dependency order and materialize outputs. +Reconcile configured Scriptorium artifacts, execute only required work in +dependency order, and safely materialize validated outputs. ## Inputs @@ -21,23 +22,24 @@ Supported source families: ## Outputs -- one materialized output per executed configured artifact (`output_path`) +- one current per-artifact manifest record per validated materialized output - stage metadata describing selected/generated/reused artifacts ## Key Behavior -- when Scriptorium is absent or no configured artifact is executable, completes - successfully with no outputs and records explanatory metadata. This is not an - explicit self-skip: both manifests record success, satisfy publish's - prerequisite, and an ordinary later run reuses the result until forced. +- when `pipeline.scriptorium` is absent or no configured artifact is + executable, completes successfully with no outputs and records explanatory + metadata. This is not an explicit self-skip: both manifests record success, + satisfy publish's prerequisite, and an ordinary later run reuses the result + until forced. - builds a runtime artifact catalog containing built-ins, configured artifacts, and configured extraction lanes. Extraction availability is hydrated only from compatible successful extraction evidence. - uses enabled configured artifacts by default. An explicit `--artifacts` - selection is a one-invocation override: it makes exactly the named configured - artifacts executable even when disabled, and does not automatically include - dependencies. A selected artifact's dependencies must instead already be - available to the catalog. + selection is a one-invocation override that makes exactly the named + configured artifacts explicit targets even when disabled. The work planner + adds required configured prerequisites, reuses current ones, and schedules + stale, missing, or otherwise non-current prerequisites before dependents. - makes a non-executable configured artifact reusable only when its current manifest record and durable output pass the configured-artifact evidence contract; an incidental or stale canonical file is unavailable. @@ -86,6 +88,29 @@ Supported source families: projected record collection. Valid unrelated configured records survive the projection, removed records are omitted, and legacy files never become current without regeneration. +- executes only the work plan's scheduled entries. Manifest-validated current + prerequisites remain available through the runtime catalog without invoking + Scriptorium; newly produced prerequisites enter that catalog with the same + contract, checksum, and size identity used for persisted current evidence. +- keeps adapter output in the invocation's run-local analyze directory until + it is a safe, non-empty, bounded regular file with a calculated checksum and + complete output contract. Canonical replacement uses the shared atomic file + operation boundary and verifies that the installed checksum matches the + validated run-local bytes. +- records each successful artifact's freshly computed fingerprint, canonical + relative output path, contract, checksum, size, producer run ID, bounded + Scriptorium provenance, logs, and generated configuration references in the + analyze-owned projection. +- preserves valid unrelated current records during partial execution. If a + rebuilt output's bytes and contract are unchanged, unselected dependents may + remain current. If that semantic identity changes, unselected transitive + dependents become stale without being executed; dependents included in the + invocation are evaluated in dependency order instead. +- reports all evaluated targets and prerequisites in invocation state. The + runner reconstructs aggregate session outputs from every current session + record and invocation outputs from only records produced by the current run. + Unrelated stale records do not make an otherwise successful partial + invocation fail. - resolves previous-session sources from local `previous/` cache only. - runs optional render-debug, then artifact execution. - validates non-empty output files and materializes canonical outputs. @@ -112,6 +137,8 @@ Supported source families: visible only inside Scriptorium—such as a file privately loaded by its config path—requires an explicit forced regeneration. - output provenance and metadata are deterministic per execution. +- a canonical file without current per-artifact manifest evidence is never + promoted to current state. ## Related Contracts And Tests @@ -126,4 +153,5 @@ Supported source families: `internal/stage/analyze_fingerprint_test.go`, `internal/stage/analyze_reconciliation.go`, and `internal/stage/analyze_reconciliation_test.go`, - `internal/stage/analyze_plan.go`, and `internal/stage/analyze_plan_test.go` + `internal/stage/analyze_plan.go`, `internal/stage/analyze_plan_test.go`, and + `internal/stage/analyze_incremental_execution_test.go` diff --git a/docs/operations.md b/docs/operations.md index 8f51bc4..a37743c 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -165,7 +165,8 @@ or `publish`. Selection behavior: - validates names against `pipeline.scriptorium.artifacts`; -- filters analyze execution to selected configured artifacts; +- selects explicit analyze targets and permits their required configured + prerequisites to be reused or rebuilt first; - filters publish rules for `narratio.artifact.` sources only; - does not suppress built-in transcript, bounds, or explicitly configured `narratio.extraction.` publish sources; and diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 7d0b3aa..47cf634 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -584,6 +584,8 @@ a deterministic artifact execution plan without invoking Scriptorium. ## Stage 12 — Incremental Analyze Execution And Promotion +**Status: Completed** + ### Goal Execute the Stage 11 work plan on the successful path, safely promote validated diff --git a/internal/artifacts/catalog.go b/internal/artifacts/catalog.go index 33f25c3..edc4de1 100644 --- a/internal/artifacts/catalog.go +++ b/internal/artifacts/catalog.go @@ -291,6 +291,41 @@ func (c *ArtifactCatalog) MarkAvailableGenerated(sourceID, path string) error { return c.markAvailable(sourceID, path, ArtifactProvenanceGeneratedCurrentAnalyzeRun) } +// MarkAvailableGeneratedEvidence marks one source as available from the +// current analyze invocation and retains the semantic output identity needed +// by later scheduled dependents. +func (c *ArtifactCatalog) MarkAvailableGeneratedEvidence( + sourceID, path, producerRunID, checksum string, + size int64, + contract *artifactmodel.ContractMetadata, +) error { + producerRunID = strings.TrimSpace(producerRunID) + checksum = strings.TrimSpace(checksum) + if err := ValidateRunIdentity(producerRunID); err != nil { + return fmt.Errorf("generated artifact producer run id: %w", err) + } + if err := validateSHA256(checksum); err != nil { + return fmt.Errorf("generated artifact checksum: %w", err) + } + if size <= 0 { + return fmt.Errorf("generated artifact size must be positive") + } + if contract == nil || strings.TrimSpace(contract.MediaType) == "" || + strings.TrimSpace(contract.SchemaID) == "" || strings.TrimSpace(contract.SchemaVersion) == "" { + return fmt.Errorf("generated artifact contract is incomplete") + } + if err := c.markAvailable(sourceID, path, ArtifactProvenanceGeneratedCurrentAnalyzeRun); err != nil { + return err + } + entry := c.entries[strings.TrimSpace(sourceID)] + entry.ProducerRunID = producerRunID + entry.Checksum = checksum + entry.Size = size + entry.Contract = cloneArtifactContract(contract) + c.entries[entry.SourceID] = entry + return nil +} + func (c *ArtifactCatalog) markAvailableFromExtractManifest( sourceID, path, producerRunID, checksum string, size int64, diff --git a/internal/artifacts/catalog_test.go b/internal/artifacts/catalog_test.go index 8254bd5..522e0fd 100644 --- a/internal/artifacts/catalog_test.go +++ b/internal/artifacts/catalog_test.go @@ -1,6 +1,11 @@ package artifacts -import "testing" +import ( + "strings" + "testing" + + "gitea.maximumdirect.net/eric/narratio/internal/artifactmodel" +) func TestArtifactCatalogRegisterBuiltInsAndLookup(t *testing.T) { catalog := NewArtifactCatalog() @@ -179,6 +184,30 @@ func TestArtifactCatalogMarkAvailableGenerated(t *testing.T) { } } +func TestArtifactCatalogMarkAvailableGeneratedEvidence(t *testing.T) { + catalog := NewArtifactCatalog() + if err := catalog.RegisterConfiguredArtifacts(map[string]ConfiguredArtifactDefinition{ + "session_recap": {Enabled: true, OutputPath: "artifacts/session_recap.md"}, + }); err != nil { + t.Fatal(err) + } + sourceID, _ := catalog.SourceIDForConfiguredKey("session_recap") + contract := &artifactmodel.ContractMetadata{ + MediaType: "text/markdown", SchemaID: "narratio.session_recap", SchemaVersion: "1", + } + checksum := strings.Repeat("a", 64) + if err := catalog.MarkAvailableGeneratedEvidence( + sourceID, "/tmp/session_recap.md", "run-1", checksum, 42, contract, + ); err != nil { + t.Fatal(err) + } + entry, _ := catalog.Lookup(sourceID) + if !entry.Available || entry.ProducerRunID != "run-1" || entry.Checksum != checksum || entry.Size != 42 || + entry.Contract == nil || *entry.Contract != *contract { + t.Fatalf("generated evidence entry = %#v", entry) + } +} + func TestArtifactCatalogLookupPlannedButUnavailable(t *testing.T) { catalog := NewArtifactCatalog() if err := catalog.RegisterConfiguredArtifacts( diff --git a/internal/stage/analyze.go b/internal/stage/analyze.go index a08bea9..8ccd8b5 100644 --- a/internal/stage/analyze.go +++ b/internal/stage/analyze.go @@ -2,8 +2,9 @@ package stage import ( "context" + "crypto/sha256" + "encoding/hex" "encoding/json" - "errors" "fmt" "path/filepath" "sort" @@ -11,6 +12,7 @@ import ( "time" "gitea.maximumdirect.net/eric/narratio/internal/adapters/scriptorium" + "gitea.maximumdirect.net/eric/narratio/internal/artifactmodel" "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" @@ -28,6 +30,8 @@ type analyzeArtifactExecutionPlan struct { type analyzeArtifactExecutionResult struct { Output artifacts.Ref + OutputSize int64 + Scriptorium manifest.AnalyzeArtifactProvenance Logs []string GeneratedConfigs []string Metadata map[string]any @@ -54,10 +58,6 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S if env.Config.Pipeline == nil || env.Config.Session == nil { return nil, fmt.Errorf("analyze: resolved config must include pipeline and session") } - if env.Scriptorium == nil { - return nil, fmt.Errorf("analyze: scriptorium adapter is required") - } - var sessionID string if m != nil { sessionID = strings.TrimSpace(m.SessionID) @@ -81,6 +81,13 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S "reason": "pipeline.scriptorium is not configured", }}, nil } + if len(env.Config.Pipeline.Scriptorium.Artifacts) == 0 { + return &StageResult{Metadata: map[string]any{ + "stage": "analyze", + "skipped": true, + "reason": "no scriptorium artifacts configured", + }}, nil + } effective := env.EffectiveArtifacts if !effective.Resolved() { @@ -104,15 +111,11 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S return nil, fmt.Errorf("analyze: build runtime artifact catalog: %w", err) } - plans, skipReason, err := buildAnalyzeExecutionPlans(env.Config.Pipeline.Scriptorium, effective, runtimeCatalog) - if err != nil { - return nil, fmt.Errorf("analyze: %w", err) - } - if skipReason != "" { + if len(effective.Keys()) == 0 { return &StageResult{Metadata: map[string]any{ "stage": "analyze", "skipped": true, - "reason": skipReason, + "reason": "no selected scriptorium artifacts to execute", }}, nil } @@ -125,21 +128,64 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S TranscriptRefs: discoverAnalyzeTranscriptRefs(m, paths), Catalog: runtimeCatalog, } + reconciliation, err := reconcileAnalyzeArtifacts(env.Config.Pipeline.Scriptorium, execution) + if err != nil { + return nil, fmt.Errorf("analyze: reconcile configured artifacts: %w", err) + } + workPlan, err := planAnalyzeWork( + env.Config.Pipeline.Scriptorium, + env.SelectedArtifactKeys, + env.Force, + reconciliation, + ) + if err != nil { + return nil, fmt.Errorf("analyze: plan configured artifacts: %w", err) + } + if len(workPlan.ExecutionOrder) > 0 && env.Scriptorium == nil { + return nil, fmt.Errorf("analyze: scriptorium adapter is required") + } - outputs := make([]artifacts.Ref, 0, len(plans)) logs := []string{} generatedConfigs := []string{} - artifactMetadata := make([]map[string]any, 0, len(plans)) + artifactMetadata := make([]map[string]any, 0, len(workPlan.ExecutionOrder)) reusedArtifacts := []map[string]any{} reusedSeen := map[string]struct{}{} + sessionRecords := manifest.CloneAnalyzeArtifactCollection(workPlan.ProjectedRecords) + invocationRecords := make(map[string]manifest.AnalyzeArtifactRecord) + priorCurrentRecords := make(map[string]manifest.AnalyzeArtifactRecord) + for _, item := range reconciliation.Ordered { + if item.Stored != nil && item.Stored.Status == manifest.AnalyzeArtifactCurrent { + priorCurrentRecords[item.Key] = *item.Stored + } + } + invocationKeys := make(map[string]struct{}, len(workPlan.ExecutionOrder)+len(workPlan.ReusedCurrent)) + for _, item := range workPlan.ExecutionOrder { + invocationKeys[item.Key] = struct{}{} + } + for _, item := range workPlan.ReusedCurrent { + invocationKeys[item.Key] = struct{}{} + if record, ok := sessionRecords[item.Key]; ok { + invocationRecords[item.Key] = record + } + } - for _, plan := range plans { + for _, item := range workPlan.ExecutionOrder { + artifactCfg := env.Config.Pipeline.Scriptorium.Artifacts[item.Key] + fingerprint, _, _, err := computeAnalyzeArtifactFingerprint( + item.Key, + env.Config.Pipeline.Scriptorium, + execution, + ) + if err != nil { + return nil, fmt.Errorf("analyze: compute execution fingerprint for artifact %q: %w", item.Key, err) + } + priorRecord, hadPriorRecord := priorCurrentRecords[item.Key] + plan := analyzeArtifactExecutionPlan{Name: item.Key, Cfg: artifactCfg} artifactResult, err := executeAnalyzeArtifact(ctx, execution, plan) if err != nil { return nil, err } - outputs = append(outputs, artifactResult.Output) logs = append(logs, artifactResult.Logs...) generatedConfigs = append(generatedConfigs, artifactResult.GeneratedConfigs...) artifactMetadata = append(artifactMetadata, artifactResult.Metadata) @@ -154,18 +200,47 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S reusedArtifacts = append(reusedArtifacts, reused) } + record, err := currentAnalyzeArtifactRecord( + execution, + plan, + fingerprint, + artifactResult, + ) + if err != nil { + return nil, fmt.Errorf("analyze: record artifact %q: %w", plan.Name, err) + } + sessionRecords[plan.Name] = record + invocationRecords[plan.Name] = record + sourceID, ok := runtimeCatalog.SourceIDForConfiguredKey(plan.Name) if !ok { return nil, fmt.Errorf("analyze: source id not found for artifact %q", plan.Name) } - if err := runtimeCatalog.MarkAvailableGenerated(sourceID, artifactResult.Output.AbsolutePath); err != nil { + if err := runtimeCatalog.MarkAvailableGeneratedEvidence( + sourceID, + artifactResult.Output.AbsolutePath, + record.ProducerRunID, + record.Output.Checksum, + record.OutputSize, + record.Output.Contract, + ); err != nil { return nil, fmt.Errorf("analyze: mark generated artifact %q available: %w", sourceID, err) } + if !hadPriorRecord || !sameAnalyzeOutputIdentity(priorRecord, record) { + staleUnscheduledAnalyzeDependents( + env.Config.Pipeline.Scriptorium.Artifacts, + plan.Name, + invocationKeys, + sessionRecords, + ) + } } metadata := map[string]any{ "stage": "analyze", - "selected_artifacts": extractPlanNames(plans), + "selected_artifacts": append([]string(nil), workPlan.ExplicitTargets...), + "executed_artifacts": analyzePlanKeysForMetadata(workPlan.ExecutionOrder), + "reused_current": analyzePlanKeysForMetadata(workPlan.ReusedCurrent), "generated_artifacts": artifactMetadata, "reused_artifacts": reusedArtifacts, "artifact_count": len(artifactMetadata), @@ -178,133 +253,134 @@ func (analyzeStage) Run(ctx context.Context, env *Env, m *manifest.Manifest) (*S } return &StageResult{ - Outputs: outputs, Logs: dedupeAndSortPaths(logs), GeneratedConfigs: dedupeAndSortPaths(generatedConfigs), Metadata: metadata, + AnalyzeState: &AnalyzeStateProjection{ + Session: sessionRecords, + Invocation: manifest.CloneAnalyzeArtifactCollection(invocationRecords), + }, }, nil } -func buildAnalyzeExecutionPlans( - scriptoriumCfg *config.ScriptoriumConfig, - effective artifacts.EffectiveArtifactSet, - catalog *artifacts.ArtifactCatalog, -) ([]analyzeArtifactExecutionPlan, string, error) { - if scriptoriumCfg == nil { - return nil, "pipeline.scriptorium is not configured", nil +func currentAnalyzeArtifactRecord( + execution analyzeExecutionContext, + plan analyzeArtifactExecutionPlan, + fingerprint string, + result *analyzeArtifactExecutionResult, +) (manifest.AnalyzeArtifactRecord, error) { + if result == nil { + return manifest.AnalyzeArtifactRecord{}, fmt.Errorf("execution result is required") } - if len(scriptoriumCfg.Artifacts) == 0 { - return nil, "no scriptorium artifacts configured", nil + producerRunID := "" + if execution.Manifest != nil { + producerRunID = strings.TrimSpace(execution.Manifest.RunID) } - - if len(effective.Keys()) == 0 { - return nil, "no selected scriptorium artifacts to execute", nil + if producerRunID == "" { + // Direct stage callers predate invocation manifests. Application-owned + // execution always supplies the actual run identity. + producerRunID = "direct-analyze" } - - ordered, err := orderSelectedScriptoriumArtifacts(scriptoriumCfg.Artifacts, effective, catalog) + relativePath, err := normalizedAnalyzeOutputIdentity(plan.Cfg.OutputPath) if err != nil { - return nil, "", err + return manifest.AnalyzeArtifactRecord{}, err } - - plans := make([]analyzeArtifactExecutionPlan, 0, len(ordered)) - for _, name := range ordered { - artifactCfg, ok := scriptoriumCfg.Artifacts[name] - if !ok { - return nil, "", fmt.Errorf("selected artifact %q is not configured", name) - } - plans = append(plans, analyzeArtifactExecutionPlan{Name: name, Cfg: artifactCfg}) + if relativePath == "" { + return manifest.AnalyzeArtifactRecord{}, fmt.Errorf("configured output path is required") } - return plans, "", nil + contract := result.Output.Contract + if contract == nil { + return manifest.AnalyzeArtifactRecord{}, fmt.Errorf("validated output contract is required") + } + record := manifest.AnalyzeArtifactRecord{ + Key: plan.Name, + Status: manifest.AnalyzeArtifactCurrent, + FingerprintVersion: manifest.AnalyzeFingerprintContractVersion, + Fingerprint: fingerprint, + Dependencies: normalizedAnalyzeDependencyKeys(plan.Cfg.DependsOn), + Output: &manifest.ArtifactRecord{ + Kind: "scriptorium_artifact", + SourceID: artifacts.ConfiguredArtifactSourceID(plan.Name), + LocalPath: relativePath, + Contract: cloneAnalyzeOutputContract(contract), + ProducerRunID: producerRunID, + Checksum: result.Output.Checksum, + }, + OutputSize: result.OutputSize, + ProducerRunID: producerRunID, + UpdatedAt: time.Now().UTC(), + Scriptorium: &result.Scriptorium, + Logs: dedupeAndSortPaths(result.Logs), + GeneratedConfigs: dedupeAndSortPaths(result.GeneratedConfigs), + } + if err := manifest.ValidateAnalyzeArtifactCollection( + manifest.AnalyzeStateContractVersion, + map[string]manifest.AnalyzeArtifactRecord{plan.Name: record}, + ); err != nil { + return manifest.AnalyzeArtifactRecord{}, err + } + return record, nil } -func orderSelectedScriptoriumArtifacts( - artifactsCfg map[string]config.ScriptoriumArtifactConfig, - effective artifacts.EffectiveArtifactSet, - catalog *artifacts.ArtifactCatalog, -) ([]string, error) { - selectedSet := map[string]struct{}{} - selected := effective.Keys() - for _, key := range selected { - selectedSet[key] = struct{}{} +func sameAnalyzeOutputIdentity(left, right manifest.AnalyzeArtifactRecord) bool { + if left.Status != manifest.AnalyzeArtifactCurrent || right.Status != manifest.AnalyzeArtifactCurrent || + left.Output == nil || right.Output == nil || left.Output.Contract == nil || right.Output.Contract == nil { + return false } + return left.OutputSize == right.OutputSize && + left.Output.Checksum == right.Output.Checksum && + *left.Output.Contract == *right.Output.Contract +} - dependencyErrors := []string{} - for _, selectedKey := range selected { - cfg, ok := artifactsCfg[selectedKey] - if !ok { - dependencyErrors = append(dependencyErrors, fmt.Sprintf("selected artifact %q is not configured", selectedKey)) +func staleUnscheduledAnalyzeDependents( + configured map[string]config.ScriptoriumArtifactConfig, + changed string, + invocationKeys map[string]struct{}, + records map[string]manifest.AnalyzeArtifactRecord, +) { + reverse := make(map[string][]string, len(configured)) + for key, artifactCfg := range configured { + for _, dependency := range normalizedAnalyzeDependencyKeys(artifactCfg.DependsOn) { + reverse[dependency] = append(reverse[dependency], key) + } + } + for key := range reverse { + sort.Strings(reverse[key]) + } + queue := append([]string(nil), reverse[changed]...) + seen := make(map[string]struct{}, len(queue)) + for len(queue) > 0 { + key := queue[0] + queue = queue[1:] + if _, visited := seen[key]; visited { continue } - for _, dep := range cfg.DependsOn { - trimmedDep := strings.TrimSpace(dep) - if trimmedDep == "" { - continue - } - if _, ok := selectedSet[trimmedDep]; ok { - continue - } - sourceID, ok := catalog.SourceIDForConfiguredKey(trimmedDep) - if !ok { - dependencyErrors = append(dependencyErrors, fmt.Sprintf("artifact %q depends on unknown configured artifact %q", selectedKey, trimmedDep)) - continue - } - entry, ok := catalog.Lookup(sourceID) - if !ok || !entry.Available { - dependencyErrors = append(dependencyErrors, fmt.Sprintf("artifact %q depends on %q, but %q is unavailable", selectedKey, trimmedDep, sourceID)) - } + seen[key] = struct{}{} + if _, evaluated := invocationKeys[key]; evaluated { + continue } + staleProjectedAnalyzeRecord(records, key) + queue = append(queue, reverse[key]...) } - if len(dependencyErrors) > 0 { - return nil, errors.New(strings.Join(dependencyErrors, "; ")) - } +} - indegree := map[string]int{} - edges := map[string][]string{} - for _, key := range selected { - indegree[key] = 0 +func analyzePlanKeysForMetadata(items []analyzePlanItem) []string { + if len(items) == 0 { + return nil } - for _, key := range selected { - cfg := artifactsCfg[key] - for _, dep := range cfg.DependsOn { - trimmedDep := strings.TrimSpace(dep) - if _, ok := selectedSet[trimmedDep]; !ok { - continue - } - edges[trimmedDep] = append(edges[trimmedDep], key) - indegree[key]++ - } + keys := make([]string, 0, len(items)) + for _, item := range items { + keys = append(keys, item.Key) } + return keys +} - for key := range edges { - sort.Strings(edges[key]) +func cloneAnalyzeOutputContract(value *artifactmodel.ContractMetadata) *artifactmodel.ContractMetadata { + if value == nil { + return nil } - - ready := make([]string, 0, len(indegree)) - for key, degree := range indegree { - if degree == 0 { - ready = append(ready, key) - } - } - sort.Strings(ready) - - order := make([]string, 0, len(selectedSet)) - for len(ready) > 0 { - node := ready[0] - ready = ready[1:] - order = append(order, node) - for _, dep := range edges[node] { - indegree[dep]-- - if indegree[dep] == 0 { - ready = append(ready, dep) - sort.Strings(ready) - } - } - } - - if len(order) != len(selectedSet) { - return nil, fmt.Errorf("selected scriptorium artifacts contain a dependency cycle") - } - return order, nil + cloned := *value + return &cloned } func executeAnalyzeArtifact( @@ -504,14 +580,32 @@ func executeAnalyzeArtifact( if err := requireNonEmptyFile(finalOutputPath, artifactName+" output"); err != nil { return nil, fmt.Errorf("analyze: %w", err) } + validatedOutput, err := readExternalResult(finalOutputPath, artifactName+" output") + if err != nil { + return nil, fmt.Errorf("analyze: %w", err) + } + contract := &artifactmodel.ContractMetadata{ + MediaType: "text/markdown", SchemaID: "narratio." + artifactName, SchemaVersion: "1", + } + relativeOutputPath, err := normalizedAnalyzeOutputIdentity(artifactCfg.OutputPath) + if err != nil { + return nil, fmt.Errorf("analyze: normalize output identity for artifact %q: %w", artifactName, err) + } materializedArtifact, err := materializeRunLocalOutput(env.ArtifactStore, finalOutputPath, canonicalOutputPath, artifacts.Ref{ - Kind: artifactName, - Category: "artifacts", - SessionID: sessionID, + Kind: artifactName, + SourceID: artifacts.ConfiguredArtifactSourceID(artifactName), + Category: "artifacts", + SessionID: sessionID, + RelativePath: relativeOutputPath, + Contract: contract, }) if err != nil { return nil, fmt.Errorf("analyze: materialize artifact output for %q: %w", artifactName, err) } + expectedDigest := sha256.Sum256(validatedOutput) + if materializedArtifact.Checksum != hex.EncodeToString(expectedDigest[:]) { + return nil, fmt.Errorf("analyze: materialized artifact output for %q differs from validated run-local bytes", artifactName) + } logPaths = append(logPaths, stdoutLogPath, stderrLogPath) generatedConfigs = append(generatedConfigs, generatedConfigPath) @@ -537,7 +631,11 @@ func executeAnalyzeArtifact( } return &analyzeArtifactExecutionResult{ - Output: materializedArtifact, + Output: materializedArtifact, + OutputSize: int64(len(validatedOutput)), + Scriptorium: manifest.AnalyzeArtifactProvenance{ + PromptID: artifactCfg.PromptID, ProfileID: artifactCfg.ProfileID, CommandMode: res.CommandMode, + }, Logs: logPaths, GeneratedConfigs: generatedConfigs, Metadata: meta, @@ -545,17 +643,6 @@ func executeAnalyzeArtifact( }, nil } -func extractPlanNames(plans []analyzeArtifactExecutionPlan) []string { - if len(plans) == 0 { - return nil - } - out := make([]string, 0, len(plans)) - for _, plan := range plans { - out = append(out, plan.Name) - } - return out -} - func configuredArtifactNameFromSourceID(sourceID string) string { name, _ := artifactpolicy.ParseConfiguredSource(sourceID) return name diff --git a/internal/stage/analyze_incremental_execution_test.go b/internal/stage/analyze_incremental_execution_test.go new file mode 100644 index 0000000..cc6a94d --- /dev/null +++ b/internal/stage/analyze_incremental_execution_test.go @@ -0,0 +1,249 @@ +package stage + +import ( + "context" + "path/filepath" + "reflect" + "testing" + + "gitea.maximumdirect.net/eric/narratio/internal/config" + "gitea.maximumdirect.net/eric/narratio/internal/manifest" +) + +func TestAnalyzeReusesCurrentArtifactWithoutScriptorium(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := sessionPathsForEnv(env, m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`) + + first, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + installAnalyzeProjection(m, first.AnalyzeState) + if len(fake.RunRequests) != 1 { + t.Fatalf("initial requests = %d, want 1", len(fake.RunRequests)) + } + + env.Scriptorium = nil + second, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatalf("reuse Run() error = %v", err) + } + if got, _ := second.Metadata["executed_artifacts"].([]string); len(got) != 0 { + t.Fatalf("executed artifacts = %#v, want none", got) + } + if got := second.Metadata["reused_current"]; !reflect.DeepEqual(got, []string{"session_recap"}) { + t.Fatalf("reused current = %#v, want session_recap", got) + } + if second.AnalyzeState.Invocation["session_recap"].Status != manifest.AnalyzeArtifactCurrent { + t.Fatalf("invocation state = %#v", second.AnalyzeState.Invocation) + } +} + +func TestAnalyzePartialForcePreservesUnrelatedCurrentRecord(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := sessionPathsForEnv(env, m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`) + addAnalyzeDependentArtifact(env) + + first, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + installAnalyzeProjection(m, first.AnalyzeState) + if len(fake.RunRequests) != 2 { + t.Fatalf("initial requests = %d, want 2", len(fake.RunRequests)) + } + priorDependent := m.Stages["analyze"].AnalyzeArtifacts["player_handout"] + + env.SelectedArtifactKeys = []string{"session_recap"} + env.Force = true + secondRunner := &orderedScriptoriumRunner{RunBody: "scriptorium noop/fake run artifact\n"} + env.Scriptorium = secondRunner + second, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + if got := secondRunner.Calls; !reflect.DeepEqual(got, []string{"run"}) { + t.Fatalf("calls = %#v, want one forced target run", got) + } + if got := second.AnalyzeState.Session["player_handout"]; !reflect.DeepEqual(got, priorDependent) { + t.Fatalf("unrelated dependent changed = %#v, want %#v", got, priorDependent) + } + if _, attempted := second.AnalyzeState.Invocation["player_handout"]; attempted { + t.Fatal("unselected dependent appeared in invocation state") + } +} + +func TestAnalyzeExposesReusedPrerequisiteToScheduledDependent(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := sessionPathsForEnv(env, m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`) + addAnalyzeDependentArtifact(env) + + first, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + installAnalyzeProjection(m, first.AnalyzeState) + dependent := env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] + dependent.PromptID = "dnd.player_handout.revised" + env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] = dependent + env.SelectedArtifactKeys = []string{"player_handout"} + fake.RunRequests = nil + + second, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + if len(fake.RunRequests) != 1 || fake.RunRequests[0].PromptID != "dnd.player_handout.revised" { + t.Fatalf("requests = %#v, want only revised dependent", fake.RunRequests) + } + if got := fake.RunRequests[0].InputPaths["recap"]; got != filepath.Join(paths.ArtifactsDir, "session_recap.md") { + t.Fatalf("reused prerequisite input = %q", got) + } + if second.AnalyzeState.Invocation["session_recap"].Status != manifest.AnalyzeArtifactCurrent || + second.AnalyzeState.Invocation["player_handout"].Status != manifest.AnalyzeArtifactCurrent { + t.Fatalf("invocation records = %#v, want reused prerequisite and produced dependent", second.AnalyzeState.Invocation) + } +} + +func TestAnalyzeChangedOutputStalesUnselectedDependents(t *testing.T) { + env, m, _ := setupAnalyzeEnv(t) + paths := sessionPathsForEnv(env, m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`) + addAnalyzeDependentArtifact(env) + + first, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + installAnalyzeProjection(m, first.AnalyzeState) + + env.SelectedArtifactKeys = []string{"session_recap"} + env.Force = true + env.Scriptorium = &orderedScriptoriumRunner{RunBody: "changed recap\n"} + second, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + dependent := second.AnalyzeState.Session["player_handout"] + if dependent.Status != manifest.AnalyzeArtifactStale || dependent.Output != nil || dependent.OutputSize != 0 { + t.Fatalf("dependent record = %#v, want stale without output", dependent) + } + if _, attempted := second.AnalyzeState.Invocation["player_handout"]; attempted { + t.Fatal("changed-output invalidation executed the unselected dependent") + } +} + +func TestAnalyzeRebuildsNestedStalePrerequisitesInDependencyOrder(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := sessionPathsForEnv(env, m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`) + addAnalyzeDependentArtifact(env) + env.Config.Pipeline.Scriptorium.Artifacts["quest_log"] = config.ScriptoriumArtifactConfig{ + Enabled: false, DependsOn: []string{"player_handout"}, PromptID: "dnd.quest_log", + OutputPath: "artifacts/quest_log.md", + Inputs: map[string]config.ScriptoriumInputConfig{ + "handout": {Source: "narratio.artifact.player_handout", Required: true}, + }, + } + + first, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + installAnalyzeProjection(m, first.AnalyzeState) + if len(fake.RunRequests) != 2 { + t.Fatalf("initial enabled requests = %d, want 2", len(fake.RunRequests)) + } + stale := m.Stages["analyze"].AnalyzeArtifacts["session_recap"] + stale.Status = manifest.AnalyzeArtifactStale + stale.Output = nil + stale.OutputSize = 0 + m.Stages["analyze"].AnalyzeArtifacts["session_recap"] = stale + + env.SelectedArtifactKeys = []string{"quest_log"} + fake.RunRequests = nil + second, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + gotPrompts := make([]string, 0, len(fake.RunRequests)) + for _, request := range fake.RunRequests { + gotPrompts = append(gotPrompts, request.PromptID) + } + wantPrompts := []string{"dnd.session_recap", "dnd.player_handout", "dnd.quest_log"} + if !reflect.DeepEqual(gotPrompts, wantPrompts) { + t.Fatalf("prompt order = %#v, want %#v", gotPrompts, wantPrompts) + } + for _, key := range []string{"session_recap", "player_handout", "quest_log"} { + if second.AnalyzeState.Session[key].Status != manifest.AnalyzeArtifactCurrent { + t.Fatalf("%s state = %#v, want current", key, second.AnalyzeState.Session[key]) + } + } +} + +func TestAnalyzeLegacyExecutionPromotesOnlyEffectiveArtifacts(t *testing.T) { + for _, test := range []struct { + name string + selected []string + wantPrompts []string + }{ + {name: "full selection", wantPrompts: []string{"dnd.player_handout", "dnd.session_recap"}}, + {name: "partial selection", selected: []string{"session_recap"}, wantPrompts: []string{"dnd.session_recap"}}, + } { + t.Run(test.name, func(t *testing.T) { + env, m, fake := setupAnalyzeEnv(t) + paths := sessionPathsForEnv(env, m.SessionID) + writeAnalyzeFile(t, filepath.Join(paths.TranscriptsDir, "final.trimmed.json"), `{"segments":[]}`) + writeAnalyzeFile(t, filepath.Join(paths.ArtifactsDir, "player_handout.md"), "legacy handout\n") + env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] = config.ScriptoriumArtifactConfig{ + Enabled: true, PromptID: "dnd.player_handout", OutputPath: "artifacts/player_handout.md", + } + env.SelectedArtifactKeys = test.selected + m.Stages["analyze"] = &manifest.StageRecord{ + Name: "analyze", Status: manifest.StatusSucceeded, + Outputs: []manifest.ArtifactRecord{{Kind: "player_handout", LocalPath: filepath.Join(paths.ArtifactsDir, "player_handout.md")}}, + } + + result, err := (analyzeStage{}).Run(context.Background(), env, m) + if err != nil { + t.Fatal(err) + } + gotPrompts := make([]string, 0, len(fake.RunRequests)) + for _, request := range fake.RunRequests { + gotPrompts = append(gotPrompts, request.PromptID) + } + if !reflect.DeepEqual(gotPrompts, test.wantPrompts) { + t.Fatalf("prompts = %#v, want %#v", gotPrompts, test.wantPrompts) + } + if len(result.AnalyzeState.Session) != len(test.wantPrompts) { + t.Fatalf("session records = %#v, want only regenerated effective artifacts", result.AnalyzeState.Session) + } + if len(test.selected) > 0 { + if _, promoted := result.AnalyzeState.Session["player_handout"]; promoted { + t.Fatal("legacy unselected canonical output was promoted") + } + } + }) + } +} + +func addAnalyzeDependentArtifact(env *Env) { + env.Config.Pipeline.Scriptorium.Artifacts["player_handout"] = config.ScriptoriumArtifactConfig{ + Enabled: true, DependsOn: []string{"session_recap"}, PromptID: "dnd.player_handout", + OutputPath: "artifacts/player_handout.md", + Inputs: map[string]config.ScriptoriumInputConfig{ + "recap": {Source: "narratio.artifact.session_recap", Required: true}, + }, + } +} + +func installAnalyzeProjection(m *manifest.Manifest, projection *AnalyzeStateProjection) { + if m.Stages["analyze"] == nil { + m.Stages["analyze"] = &manifest.StageRecord{Name: "analyze"} + } + m.Stages["analyze"].AnalyzeStateVersion = manifest.AnalyzeStateContractVersion + m.Stages["analyze"].AnalyzeArtifacts = manifest.CloneAnalyzeArtifactCollection(projection.Session) +} diff --git a/internal/stage/analyze_order_test.go b/internal/stage/analyze_order_test.go deleted file mode 100644 index fd2755f..0000000 --- a/internal/stage/analyze_order_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package stage - -import ( - "strings" - "testing" - - "gitea.maximumdirect.net/eric/narratio/internal/artifacts" - "gitea.maximumdirect.net/eric/narratio/internal/config" -) - -func TestOrderSelectedScriptoriumArtifactsReportsUnavailableDependenciesDeterministically(t *testing.T) { - configured := map[string]config.ScriptoriumArtifactConfig{ - "alpha": {Enabled: true, DependsOn: []string{"alpha_dep"}}, - "alpha_dep": {Enabled: false, OutputPath: "artifacts/alpha_dep.md"}, - "zeta": {Enabled: true, DependsOn: []string{"zeta_dep"}}, - "zeta_dep": {Enabled: false, OutputPath: "artifacts/zeta_dep.md"}, - } - effective, err := artifacts.ResolveEffectiveArtifactSet( - artifacts.ConfiguredArtifactDefinitions(configured), - []string{"alpha", "zeta"}, - ) - if err != nil { - t.Fatalf("ResolveEffectiveArtifactSet() error = %v", err) - } - catalog, err := artifacts.BootstrapRuntimeCatalog( - artifacts.ConfiguredArtifactDefinitions(configured), - effective, - nil, - ) - if err != nil { - t.Fatalf("BootstrapRuntimeCatalog() error = %v", err) - } - - for i := 0; i < 100; i++ { - _, err := orderSelectedScriptoriumArtifacts(configured, effective, catalog) - if err == nil { - t.Fatal("orderSelectedScriptoriumArtifacts() error = nil, want unavailable dependency") - } - want := `artifact "alpha" depends on "alpha_dep", but "narratio.artifact.alpha_dep" is unavailable; artifact "zeta" depends on "zeta_dep", but "narratio.artifact.zeta_dep" is unavailable` - if !strings.Contains(err.Error(), want) { - t.Fatalf("attempt %d error = %q, want %q", i, err, want) - } - } -} diff --git a/internal/stage/analyze_test.go b/internal/stage/analyze_test.go index a56a006..5d82d32 100644 --- a/internal/stage/analyze_test.go +++ b/internal/stage/analyze_test.go @@ -62,8 +62,8 @@ func TestAnalyzeGeneratesSessionRecapFromTrimmedTranscript(t *testing.T) { t.Fatalf("session_id var = %q, want sticky narratio session id", req.Vars["session_id"]) } - if len(result.Outputs) != 1 || result.Outputs[0].Kind != "session_recap" { - t.Fatalf("outputs = %#v, want one session_recap output", result.Outputs) + if result.AnalyzeState == nil || result.AnalyzeState.Invocation["session_recap"].Status != manifest.AnalyzeArtifactCurrent { + t.Fatalf("analyze state = %#v, want current session_recap", result.AnalyzeState) } if len(result.Logs) != 2 { t.Fatalf("logs = %#v, want stdout+stderr logs", result.Logs) @@ -314,11 +314,11 @@ func TestAnalyzeUsesRunLocalPathsAndMaterializesCanonical(t *testing.T) { if !strings.Contains(fake.RunRequests[0].OutputPath, filepath.Join("runs", m.RunID, "analyze", "outputs")) { t.Fatalf("run output path = %q, want run-local path", fake.RunRequests[0].OutputPath) } - if len(result.Outputs) != 1 { - t.Fatalf("outputs len = %d, want 1", len(result.Outputs)) + if result.AnalyzeState == nil || result.AnalyzeState.Invocation["session_recap"].Output == nil { + t.Fatalf("analyze state = %#v, want session recap output evidence", result.AnalyzeState) } - if strings.Contains(result.Outputs[0].AbsolutePath, string(filepath.Separator)+"runs"+string(filepath.Separator)) { - t.Fatalf("materialized output path = %q, want canonical session path", result.Outputs[0].AbsolutePath) + if err := requireNonEmptyFile(filepath.Join(paths.ArtifactsDir, "session_recap.md"), "materialized output"); err != nil { + t.Fatalf("canonical output = %v", err) } } @@ -682,8 +682,8 @@ func TestAnalyzeAppliesSelectedArtifactsFilter(t *testing.T) { if fake.RunRequests[0].PromptID != "dnd.player_handout" { t.Fatalf("prompt id = %q, want dnd.player_handout", fake.RunRequests[0].PromptID) } - if len(result.Outputs) != 1 || result.Outputs[0].Kind != "player_handout" { - t.Fatalf("outputs = %#v, want only player_handout", result.Outputs) + if result.AnalyzeState == nil || len(result.AnalyzeState.Invocation) != 1 || result.AnalyzeState.Invocation["player_handout"].Status != manifest.AnalyzeArtifactCurrent { + t.Fatalf("analyze state = %#v, want only current player_handout", result.AnalyzeState) } } @@ -1389,11 +1389,9 @@ func TestAnalyzeRecordsRefsAndMetadata(t *testing.T) { if err != nil { t.Fatalf("Run() error = %v", err) } - if len(result.Outputs) != 1 { - t.Fatalf("outputs len = %d, want 1", len(result.Outputs)) - } - if result.Outputs[0].AbsolutePath != filepath.Join(paths.ArtifactsDir, "session_recap.md") { - t.Fatalf("output path = %q, want session recap path", result.Outputs[0].AbsolutePath) + record, ok := result.AnalyzeState.Invocation["session_recap"] + if !ok || record.Output == nil || record.Output.LocalPath != "artifacts/session_recap.md" { + t.Fatalf("analyze output record = %#v, want session recap evidence", record) } if len(result.Logs) != 2 { t.Fatalf("logs = %#v, want two logs", result.Logs)