diff --git a/.triage/102/triage.md b/.triage/102/triage.md new file mode 100644 index 0000000..c6ca9d8 --- /dev/null +++ b/.triage/102/triage.md @@ -0,0 +1,88 @@ +# Triage: Issue #102 + +> feat: add synthesis layer diagnostics to dewey doctor + +## Verdict + +| Dimension | Result | +|---|---| +| **Verdict** | VALID (5/5 unanimous) | +| **Category** | feature | +| **Objectivity** | objective (5/5 unanimous) | +| **Split** | none (5/5 unanimous) | +| **Duplicates** | none found | +| **Confidence** | HIGH | +| **Scope** | small | +| **Spec path** | OpenSpec | + +## Panel Assessments + +### Adversary + +- **Verdict**: VALID (feature, objective) +- No new attack surface — extends existing diagnostic-only CLI command +- Mirrors established Embedding Layer pattern (`autoStart=false`, bounded timeouts) +- **Key concern**: Ensure OAuth tokens are never displayed for Vertex providers; report only configuration metadata (endpoint, project, region, model) +- SSRF not a concern — endpoints are locally configured by the user +- Existing `ollamaHealthCheck` with 2s timeout prevents resource exhaustion + +### Architect + +- **Verdict**: VALID (enhancement, objective) +- Strong structural symmetry with existing Embedding Layer section (cli.go:1466–1517) +- All building blocks exist: `llm.ReadSynthesisConfig()`, `ResolveSynthesisEndpoint()`, `OllamaSynthesizer.Available()`, `VertexSynthesizer.Available()` +- Maps to single function modification (`runDoctorChecks` in `cli.go`), ~30-50 lines +- Existing helpers reusable: `doctorCounter`, `printCheck`, `section()`, `ensureOllama()` +- Convention adherent: Cobra CLI, charmbracelet/log, standard library testing, no global state + +### Guard + +- **Verdict**: VALID (enhancement, objective) +- Directly supports Constitution Principle III (Observable Quality): system must be auditable +- Explicit follow-up from OpenSpec `synthesis-endpoint-env` risk R3 (deferred with "A separate issue should be filed if needed") +- Narrow, well-bounded scope — no gate modifications, no governance changes + +### SRE + +- **Verdict**: VALID (feature, objective) +- Real observability gap worsened post-#71 decoupling of synthesis/embedding endpoints +- Closes monitoring gap for `dewey compile`, `dewey curate`, and `store_compiled` MCP tool +- Minimal performance impact: one additional HTTP health check with 5s timeout +- `Available()` methods already implement caching +- Doctor is diagnostic-only (`autoStart=false`), no subprocess spawning risk + +### Tester + +- **Verdict**: VALID (feature, objective) +- HIGH testability rating +- Existing test patterns: `runDoctorChecks(w io.Writer, ...)` enables output assertions via `strings.Contains()` +- `NoopSynthesizer` test double already exists with `Avail` and `Model` fields +- Mock Ollama server pattern (`newMockOllamaServer()` in `main_test.go`) supports connectivity checks +- **Clarification areas** (implementation details, not validity concerns): + 1. Zero-config behavior: should section be omitted, INFO/PASS, or WARN? + 2. Vertex connectivity check isolation: `VertexSynthesizer.Available()` calls actual Vertex API, conflicting with Constitution IV (no external services in tests) +- LOW regression risk — additive section, does not modify existing sections + +## Implementation Guidance + +1. **Spec workflow**: OpenSpec recommended (tactical, single package, <3 user stories) +2. **Vertex connectivity**: Report config metadata only; skip live API probe (requires GCP auth, violates Constitution IV testability). Use `Available()` for Ollama; report config completeness for Vertex. +3. **Zero-config**: Gracefully report "not configured" when no synthesis provider is set +4. **Token safety**: Display endpoint, project, region, model — never OAuth tokens or credentials +5. **Test coverage**: Mirror existing embedding layer test patterns in `cli_test.go`/`main_test.go`; monitor CRAPload on `runDoctorChecks()` (~335 lines, adding ~30-50 more) +6. **Dual-endpoint probing**: If embedding and synthesis point to different Ollama instances, doctor needs two separate probes — one per endpoint + +## Recommended Labels + +- `triage/valid` +- `type/feature` +- `scope/small` +- `spec/openspec` + +## Metadata + +- **Triaged**: 2026-08-20 +- **Issue author**: jflowers +- **Assignee**: yvonnedevlinrh +- **Existing labels**: `next-release` +- **Related**: #71 (DEWEY_SYNTHESIS_ENDPOINT), OpenSpec `synthesis-endpoint-env` risk R3 diff --git a/README.md b/README.md index bedfa68..26eae7b 100644 --- a/README.md +++ b/README.md @@ -372,7 +372,7 @@ dewey status [--vault PATH] [--json] ### dewey doctor -Run diagnostic checks for Dewey dependencies and report pass/fail with fix instructions. Checks: workspace initialization, database health (per-source page counts), Ollama availability, embedding model status, MCP server process, and opencode.json configuration. +Run diagnostic checks for Dewey dependencies and report pass/fail with fix instructions. Checks: workspace initialization, database health (per-source page counts), Ollama availability, embedding model status, synthesis layer configuration (Ollama/Vertex provider, connectivity, model availability), MCP server process, and opencode.json configuration. ```bash dewey doctor [--vault PATH] diff --git a/cli.go b/cli.go index 067cb1f..6d48c02 100644 --- a/cli.go +++ b/cli.go @@ -1517,6 +1517,70 @@ func runDoctorChecks(w io.Writer, vaultPath string) { } dp("\n") + // --- Synthesis Layer --- + synthCfg := llm.ReadSynthesisConfig(deweyDir) + + switch synthCfg.Provider { + case "ollama", "": + if synthCfg.Model == "" { + // No synthesis configured — this is optional per Composability First. + section("Synthesis Layer (not configured)") + c.printCheck(w, "PASS", "synthesis", "not configured (optional)") + } else { + synthEndpoint := synthCfg.Endpoint + if synthEndpoint == "" { + synthEndpoint = llm.ResolveSynthesisEndpoint() + } + section(fmt.Sprintf("Synthesis Layer (%s via %s)", synthCfg.Model, synthEndpoint)) + + // Ollama connectivity check (autoStart=false — doctor is diagnostic-only). + synthOllamaState, _ := ensureOllama(synthEndpoint, false, nil) + synthReachable := false + // OllamaManaged is unreachable here because autoStart=false. + switch synthOllamaState { + case OllamaExternal: + synthReachable = true + c.printCheck(w, "PASS", "ollama", fmt.Sprintf("running (external) (%s)", synthEndpoint)) + case OllamaUnavailable: + if _, lookErr := exec.LookPath("ollama"); lookErr == nil { + c.printCheck(w, "WARN", "ollama", "not running") + dp(" Fix: ollama serve\n") + } else { + c.printCheck(w, "PASS", "ollama", "not installed (optional)") + } + } + + // Model availability. + if synthReachable { + synth := llm.NewOllamaSynthesizer(synthEndpoint, synthCfg.Model) + if synth.Available() { + c.printCheck(w, "PASS", "model", fmt.Sprintf("%s ready", synthCfg.Model)) + } else { + c.printCheck(w, "FAIL", "model", fmt.Sprintf("%s not available", synthCfg.Model)) + dp(" Fix: ollama pull %s\n", synthCfg.Model) + } + } else { + c.printCheck(w, "WARN", "model", fmt.Sprintf("%s skipped (ollama not reachable)", synthCfg.Model)) + } + } + + case "vertex": + section(fmt.Sprintf("Synthesis Layer (%s via vertex)", synthCfg.Model)) + + // Vertex config completeness — no live API calls (no GCP credentials required). + _, synthErr := llm.NewSynthesizerFromConfig(synthCfg) + if synthErr != nil { + c.printCheck(w, "FAIL", "config", fmt.Sprintf("incomplete: %s", synthErr)) + } else { + c.printCheck(w, "PASS", "config", fmt.Sprintf("project=%s region=%s", synthCfg.Project, synthCfg.Region)) + } + + default: + section(fmt.Sprintf("Synthesis Layer (unknown provider: %s)", synthCfg.Provider)) + c.printCheck(w, "FAIL", "provider", fmt.Sprintf("unknown provider %q (supported: ollama, vertex)", synthCfg.Provider)) + } + dp("\n") + // --- MCP Server --- section("MCP Server") diff --git a/cli_test.go b/cli_test.go index c8f1bc1..3358df6 100644 --- a/cli_test.go +++ b/cli_test.go @@ -3156,6 +3156,11 @@ func TestDoctorCmd_WithInitializedVault(t *testing.T) { t.Errorf("doctor should include Embedding Layer section, got:\n%s", output) } + // Synthesis Layer section should exist. + if !strings.Contains(output, "Synthesis Layer") { + t.Errorf("doctor should include Synthesis Layer section, got:\n%s", output) + } + // Summary box should be present with correct counts. if !strings.Contains(output, "✅") { t.Errorf("doctor should include summary box with pass emoji, got:\n%s", output) @@ -3173,6 +3178,171 @@ func TestDoctorCmd_WithInitializedVault(t *testing.T) { } } +// TestDoctorCmd_SynthesisUnconfigured verifies the Synthesis Layer section +// reports "not configured (optional)" with PASS status when no synthesis +// provider is configured. +func TestDoctorCmd_SynthesisUnconfigured(t *testing.T) { + tmpDir := t.TempDir() + deweyDir := filepath.Join(tmpDir, deweyWorkspaceDir) + if err := os.MkdirAll(deweyDir, 0o755); err != nil { + t.Fatalf("mkdir .uf/dewey: %v", err) + } + + // Clear any env vars that could configure synthesis. + t.Setenv("DEWEY_SYNTHESIS_ENDPOINT", "") + t.Setenv("DEWEY_GENERATION_MODEL", "") + t.Setenv("OLLAMA_HOST", "") + // Isolate from developer's global config (~/.config/dewey/config.yaml). + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + var buf bytes.Buffer + runDoctorChecks(&buf, tmpDir) + output := buf.String() + + if !strings.Contains(output, "Synthesis Layer (not configured)") { + t.Errorf("doctor should show 'Synthesis Layer (not configured)', got:\n%s", output) + } + if !strings.Contains(output, "not configured (optional)") { + t.Errorf("doctor should show 'not configured (optional)' PASS, got:\n%s", output) + } +} + +// TestDoctorCmd_SynthesisVertexConfigured verifies the Synthesis Layer section +// reports config completeness for a Vertex provider without requiring GCP +// credentials (passes without application-default credentials). +func TestDoctorCmd_SynthesisVertexConfigured(t *testing.T) { + tmpDir := t.TempDir() + deweyDir := filepath.Join(tmpDir, deweyWorkspaceDir) + if err := os.MkdirAll(deweyDir, 0o755); err != nil { + t.Fatalf("mkdir .uf/dewey: %v", err) + } + + // Isolate from developer's global config (~/.config/dewey/config.yaml). + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + // Write config.yaml with vertex synthesis provider. + configYAML := `synthesis: + provider: vertex + model: claude-sonnet-4-6 + project: my-test-project + region: us-east5 +` + if err := os.WriteFile(filepath.Join(deweyDir, "config.yaml"), []byte(configYAML), 0o644); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + + // Clear env vars to ensure config.yaml is the source. + t.Setenv("DEWEY_SYNTHESIS_ENDPOINT", "") + t.Setenv("DEWEY_GENERATION_MODEL", "") + t.Setenv("OLLAMA_HOST", "") + + var buf bytes.Buffer + runDoctorChecks(&buf, tmpDir) + output := buf.String() + + if !strings.Contains(output, "Synthesis Layer") { + t.Errorf("doctor should include Synthesis Layer section, got:\n%s", output) + } + if !strings.Contains(output, "vertex") { + t.Errorf("doctor should mention vertex provider, got:\n%s", output) + } + if !strings.Contains(output, "claude-sonnet-4-6") { + t.Errorf("doctor should display synthesis model, got:\n%s", output) + } + if !strings.Contains(output, "project=my-test-project") { + t.Errorf("doctor should display project, got:\n%s", output) + } + if !strings.Contains(output, "region=us-east5") { + t.Errorf("doctor should display region, got:\n%s", output) + } + + // Token safety: Vertex output MUST NOT contain credential-like strings + // (spec requirement: "Token safety in diagnostic output"). + for _, forbidden := range []string{"Bearer", "token=", "key=", "credential"} { + if strings.Contains(output, forbidden) { + t.Errorf("doctor output MUST NOT contain credential-like string %q, got:\n%s", forbidden, output) + } + } +} + +// TestDoctorCmd_SynthesisVertexMisconfigured verifies the Synthesis Layer +// section reports FAIL for a Vertex provider with missing required fields. +func TestDoctorCmd_SynthesisVertexMisconfigured(t *testing.T) { + tmpDir := t.TempDir() + deweyDir := filepath.Join(tmpDir, deweyWorkspaceDir) + if err := os.MkdirAll(deweyDir, 0o755); err != nil { + t.Fatalf("mkdir .uf/dewey: %v", err) + } + + // Isolate from developer's global config (~/.config/dewey/config.yaml). + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + // Write config.yaml with vertex provider but missing project. + configYAML := `synthesis: + provider: vertex + model: claude-sonnet-4-6 + region: us-east5 +` + if err := os.WriteFile(filepath.Join(deweyDir, "config.yaml"), []byte(configYAML), 0o644); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + + t.Setenv("DEWEY_SYNTHESIS_ENDPOINT", "") + t.Setenv("DEWEY_GENERATION_MODEL", "") + t.Setenv("OLLAMA_HOST", "") + + var buf bytes.Buffer + runDoctorChecks(&buf, tmpDir) + output := buf.String() + + if !strings.Contains(output, "Synthesis Layer") { + t.Errorf("doctor should include Synthesis Layer section, got:\n%s", output) + } + // Should report FAIL with the specific missing field. + if !strings.Contains(output, "incomplete") { + t.Errorf("doctor should report incomplete config, got:\n%s", output) + } + if !strings.Contains(output, "project") { + t.Errorf("doctor should identify 'project' as missing field, got:\n%s", output) + } +} + +// TestDoctorCmd_SynthesisUnknownProvider verifies the Synthesis Layer section +// reports FAIL for an unknown provider type. +func TestDoctorCmd_SynthesisUnknownProvider(t *testing.T) { + tmpDir := t.TempDir() + deweyDir := filepath.Join(tmpDir, deweyWorkspaceDir) + if err := os.MkdirAll(deweyDir, 0o755); err != nil { + t.Fatalf("mkdir .uf/dewey: %v", err) + } + + // Isolate from developer's global config (~/.config/dewey/config.yaml). + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + configYAML := `synthesis: + provider: badprovider + model: some-model +` + if err := os.WriteFile(filepath.Join(deweyDir, "config.yaml"), []byte(configYAML), 0o644); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + + t.Setenv("DEWEY_SYNTHESIS_ENDPOINT", "") + t.Setenv("DEWEY_GENERATION_MODEL", "") + t.Setenv("OLLAMA_HOST", "") + + var buf bytes.Buffer + runDoctorChecks(&buf, tmpDir) + output := buf.String() + + if !strings.Contains(output, "unknown provider") { + t.Errorf("doctor should report unknown provider, got:\n%s", output) + } + if !strings.Contains(output, "badprovider") { + t.Errorf("doctor should include the unknown provider name, got:\n%s", output) + } +} + // TestDoctorCmd_MissingDeweyDir verifies doctor reports fail with // `dewey init` fix when .uf/dewey/ does not exist. func TestDoctorCmd_MissingDeweyDir(t *testing.T) { diff --git a/main_test.go b/main_test.go index 600b24e..9aacdc8 100644 --- a/main_test.go +++ b/main_test.go @@ -609,6 +609,100 @@ func TestRunDoctorChecks_ResolvesOllamaHostEndpoint(t *testing.T) { } } +// TestRunDoctorChecks_SynthesisOllamaProvider verifies the Synthesis Layer +// section reports connectivity and model availability for an Ollama synthesis +// provider, using an independent endpoint from the embedding layer. +func TestRunDoctorChecks_SynthesisOllamaProvider(t *testing.T) { + // Create separate mock servers for embedding and synthesis to verify + // dual-endpoint independence (triage guidance #6). + embedSrv := newMockOllamaServer(`{"name":"granite-embedding:30m"}`) + defer embedSrv.Close() + + synthModel := "llama3.2:3b" + synthSrv := newMockOllamaServer(`{"name":"` + synthModel + `"}`) + defer synthSrv.Close() + + tmpDir := t.TempDir() + deweyDir := filepath.Join(tmpDir, ".uf", "dewey") + if err := os.MkdirAll(deweyDir, 0o755); err != nil { + t.Fatal(err) + } + + // Isolate from developer's global config (~/.config/dewey/config.yaml). + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + // Write config.yaml with ollama synthesis provider pointing to synthSrv. + configYAML := "synthesis:\n provider: ollama\n model: " + synthModel + "\n endpoint: " + synthSrv.URL + "\n" + if err := os.WriteFile(filepath.Join(deweyDir, "config.yaml"), []byte(configYAML), 0o644); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + + // Set embedding endpoint to a different server. + t.Setenv("DEWEY_EMBEDDING_ENDPOINT", embedSrv.URL) + t.Setenv("DEWEY_SYNTHESIS_ENDPOINT", "") + t.Setenv("OLLAMA_HOST", "") + + var buf bytes.Buffer + runDoctorChecks(&buf, tmpDir) + output := buf.String() + + // Verify Synthesis Layer section exists with the correct endpoint. + if !strings.Contains(output, "Synthesis Layer") { + t.Errorf("doctor should include Synthesis Layer section, got:\n%s", output) + } + if !strings.Contains(output, synthSrv.URL) { + t.Errorf("doctor synthesis section should show synthesis endpoint %q, got:\n%s", synthSrv.URL, output) + } + if !strings.Contains(output, synthModel) { + t.Errorf("doctor synthesis section should show model %q, got:\n%s", synthModel, output) + } + + // Verify embedding endpoint is independent — embedding section shows embedSrv.URL. + if !strings.Contains(output, embedSrv.URL) { + t.Errorf("doctor embedding section should show embedding endpoint %q, got:\n%s", embedSrv.URL, output) + } +} + +// TestRunDoctorChecks_SynthesisOllamaUnreachable verifies the Synthesis Layer +// section reports WARN when the Ollama synthesis endpoint is unreachable and +// skips the model availability check. +func TestRunDoctorChecks_SynthesisOllamaUnreachable(t *testing.T) { + // Create a server and immediately close it to get an unreachable endpoint. + unreachableSrv := newMockOllamaServer("") + unreachableURL := unreachableSrv.URL + unreachableSrv.Close() + + tmpDir := t.TempDir() + deweyDir := filepath.Join(tmpDir, ".uf", "dewey") + if err := os.MkdirAll(deweyDir, 0o755); err != nil { + t.Fatal(err) + } + + synthModel := "llama3.2:3b" + configYAML := "synthesis:\n provider: ollama\n model: " + synthModel + "\n endpoint: " + unreachableURL + "\n" + if err := os.WriteFile(filepath.Join(deweyDir, "config.yaml"), []byte(configYAML), 0o644); err != nil { + t.Fatalf("write config.yaml: %v", err) + } + + t.Setenv("DEWEY_SYNTHESIS_ENDPOINT", "") + t.Setenv("DEWEY_EMBEDDING_ENDPOINT", "") + t.Setenv("OLLAMA_HOST", "") + // Isolate from developer's global config (~/.config/dewey/config.yaml). + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + var buf bytes.Buffer + runDoctorChecks(&buf, tmpDir) + output := buf.String() + + if !strings.Contains(output, "Synthesis Layer") { + t.Errorf("doctor should include Synthesis Layer section, got:\n%s", output) + } + // Model check should be skipped when Ollama is not reachable. + if !strings.Contains(output, "skipped (ollama not reachable)") { + t.Errorf("doctor should report model check skipped when Ollama unreachable, got:\n%s", output) + } +} + // --- OllamaState tests (T011) --- // TestOllamaState_String verifies the String() method returns the correct diff --git a/openspec/changes/doctor-synthesis-diagnostics/.openspec.yaml b/openspec/changes/doctor-synthesis-diagnostics/.openspec.yaml new file mode 100644 index 0000000..2f8943c --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-08-20 diff --git a/openspec/changes/doctor-synthesis-diagnostics/design.md b/openspec/changes/doctor-synthesis-diagnostics/design.md new file mode 100644 index 0000000..2f5b5ef --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/design.md @@ -0,0 +1,128 @@ +## Context + +The `dewey doctor` command provides diagnostic output organized into +sections: Environment, Workspace, Database, Content Sources, Content +Sanitization, Embedding Layer, and MCP Server. Each section uses the +`doctorCounter` + `printCheck()` pattern to report pass/warn/fail/info +checks. + +Since spec 016-pluggable-providers and the synthesis-endpoint-env +OpenSpec (#71), embedding and synthesis are independently configurable +provider stacks with separate endpoints, models, and provider types. +However, `dewey doctor` only reports the Embedding Layer — leaving the +Synthesis Layer completely undiagnosed. + +The existing Embedding Layer section (`cli.go:1466-1518`) demonstrates +the exact pattern to follow: +1. Read config → `embed.ReadEmbeddingConfig(deweyDir)` +2. Section header with model + endpoint +3. Ollama connectivity → `ensureOllama(endpoint, false, nil)` +4. Model availability → `embedder.Available()` +5. Additional checks (legacy model advisory, embedding count) + +## Goals / Non-Goals + +### Goals + +- Add a "Synthesis Layer" section to `dewey doctor` between the + existing Embedding Layer and MCP Server sections +- Report resolved endpoint, provider type, model, connectivity + status, and model/credential availability +- Handle all three provider states: ollama, vertex, unconfigured +- Maintain test coverage parity with the Embedding Layer section + +### Non-Goals + +- Modifying the Synthesizer interface or adding new methods +- Adding synthesis diagnostics to the `health` MCP tool (separate + concern) +- Live API calls to Vertex AI for connectivity checks (violates + Constitution IV — no external services in tests) +- Exposing OAuth tokens or credentials in doctor output + +## Decisions + +### D1: Mirror the Embedding Layer pattern exactly + +The synthesis section will follow the same structure as the embedding +section: read config, print section header, check connectivity, +check model availability. This provides consistency for users and +minimizes implementation risk. + +**Rationale**: The embedding section is proven, tested, and familiar +to users. Diverging from the pattern would create unnecessary +cognitive load. + +### D2: Provider-specific connectivity checks + +- **Ollama provider**: Reuse `ensureOllama(synthEndpoint, false, nil)` + for connectivity, then `OllamaSynthesizer.Available()` for model + availability. This is identical to the embedding pattern. Note: when + embedding and synthesis use different Ollama endpoints, doctor + issues separate connectivity probes to each — the synthesis probe + uses the resolved synthesis endpoint, independent from embedding. +- **Vertex provider**: Report config completeness (project, region, + model set) without making a live API call. Use + `NewSynthesizerFromConfig()` — if it returns an error, required + fields are missing. If it succeeds, report "configured" status. Do + NOT call `VertexSynthesizer.Available()` from doctor because it + invokes `tokenFn(ctx)` which requires real GCP credentials. +- **Unconfigured**: When `ReadSynthesisConfig()` returns a zero-value + `ProviderConfig` (empty provider, empty model), report "not + configured (optional)" with PASS status. Synthesis is not required + for core functionality (Composability First). + +**Rationale**: Ollama connectivity is cheap (local HTTP GET). Vertex +connectivity requires OAuth credentials which may not be available in +all environments and would violate the diagnostic-only contract of +`dewey doctor`. Config completeness validation catches the most +common Vertex misconfiguration (missing project/region). + +### D3: Construct synthesizer inline (no parameter injection) + +Like the embedding section which constructs +`embed.NewOllamaEmbedder()` inline, the synthesis section will call +`llm.ReadSynthesisConfig()` + `llm.NewSynthesizerFromConfig()` +inline. No new parameters to `runDoctorChecks()`. + +**Rationale**: Consistency with existing pattern. The `NoopSynthesizer` +test double is available for integration tests but is not used for +doctor tests — since doctor constructs the synthesizer inline, +`NoopSynthesizer` cannot be injected. Tests control behavior via +environment variables, config.yaml files, and mock HTTP servers, +matching the embedding section's test pattern. + +### D4: Section position + +Insert the synthesis section immediately after the Embedding Layer +section (`cli.go:1518`) and before the MCP Server section +(`cli.go:1520`). The two provider sections are logically grouped. + +## Risks / Trade-offs + +### R1: Vertex credential validation gap (ACCEPTED) + +By not calling `VertexSynthesizer.Available()`, doctor cannot verify +that GCP application-default credentials are actually valid. Users +with correct config but expired/missing credentials will see +"configured" in doctor but fail at `dewey compile` time. This is +the same trade-off the embedding section makes for Ollama model +availability when Ollama is unreachable (it reports "skipped" rather +than failing). + +**Mitigation**: The doctor output clearly indicates it's reporting +configuration status, not full end-to-end readiness. + +### R2: Large function growth (LOW) + +`runDoctorChecks()` is ~335 lines and will grow by ~40-50 lines. +This is still within reasonable bounds for a sequential diagnostic +function. The function's complexity is linear (no branching depth +increase), so CRAPload impact is modest. + +**Mitigation**: Adequate test coverage for all branches (ollama +reachable/unreachable, vertex configured/misconfigured, unconfigured). +Consider filing a follow-up issue to extract section-level helper +functions (e.g., `checkEmbeddingLayer()`, `checkSynthesisLayer()`) if +the function exceeds ~400 lines. + diff --git a/openspec/changes/doctor-synthesis-diagnostics/proposal.md b/openspec/changes/doctor-synthesis-diagnostics/proposal.md new file mode 100644 index 0000000..4a21c14 --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/proposal.md @@ -0,0 +1,79 @@ +# Proposal: Add Synthesis Layer Diagnostics to dewey doctor + +**Issue**: [#102](https://github.com/unbound-force/dewey/issues/102) +**Provenance**: OpenSpec `synthesis-endpoint-env` risk R3 (deferred) + +## Why + +The `dewey doctor` command reports comprehensive diagnostics for the +Embedding Layer (endpoint, Ollama state, model availability, embedding +count) but has zero coverage for the Synthesis Layer. Since #71 +decoupled synthesis and embedding endpoints via `DEWEY_SYNTHESIS_ENDPOINT`, +users configuring dual-provider setups (e.g., Ollama for embedding, +Vertex AI for synthesis) have no way to verify synthesis configuration +short of attempting a `dewey compile` and observing failure. + +This gap was explicitly acknowledged during the `synthesis-endpoint-env` +OpenSpec as risk R3 and deferred with the note: "A separate issue +should be filed if needed." + +## What Changes + +Add a "Synthesis Layer" section to `dewey doctor` output, inserted +between the existing "Embedding Layer" and "MCP Server" sections in +`runDoctorChecks()`. The section mirrors the embedding diagnostics +pattern and reports: + +1. **Resolved endpoint** — the synthesis endpoint after precedence + resolution (config.yaml > DEWEY_SYNTHESIS_ENDPOINT > OLLAMA_HOST > + default) +2. **Provider type** — `ollama`, `vertex`, or unconfigured +3. **Model** — the configured synthesis model +4. **Connectivity** — for Ollama: HTTP health check via existing + `ollamaHealthCheck()`; for Vertex: credential availability check + (config completeness, no live API call) +5. **Model availability** — for Ollama: model presence via + `Available()`; for Vertex: config validation (project + model set) + +## Capabilities + +- Users can verify synthesis provider configuration without + attempting a compile or curate operation +- Dual-provider setups (different endpoints for embedding vs + synthesis) are fully diagnosable +- Zero-config state is clearly reported ("not configured") + +## Impact + +- **Files modified**: `cli.go` (~30-50 lines added to + `runDoctorChecks()`), test files +- **New packages**: none +- **New MCP tools**: none +- **Schema changes**: none +- **Breaking changes**: none — additive diagnostic output only +- **Performance**: one additional HTTP check with 5s timeout (Ollama) + or no network call (Vertex/unconfigured) + +## Constitution Alignment + +### I. Composability First — PASS + +No new dependencies introduced. Doctor continues to work with any +vault configuration. Synthesis diagnostics degrade gracefully when +no provider is configured. + +### II. Autonomous Collaboration — N/A + +No MCP tool changes. Existing tool contracts unchanged. + +### III. Observable Quality — PASS + +Directly addresses this principle. The system becomes fully auditable +for both provider stacks (embedding + synthesis). + +### IV. Testability — PASS + +Existing test patterns (`io.Writer` output assertions, +`NoopSynthesizer` double, mock Ollama server) provide complete +test templates. No external services required for testing. Vertex +connectivity check uses config validation only (no GCP auth in tests). diff --git a/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md b/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md new file mode 100644 index 0000000..dc469d5 --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md @@ -0,0 +1,103 @@ +## ADDED Requirements + +### Requirement: Synthesis Layer diagnostic section + +The `dewey doctor` command MUST include a "Synthesis Layer" section +that reports synthesis provider diagnostics. The section MUST appear +after the "Embedding Layer" section and before the "MCP Server" +section. + +The section MUST report: +1. **Provider type** — `ollama`, `vertex`, or unconfigured +2. **Resolved endpoint** — the synthesis endpoint after precedence + resolution +3. **Model** — the configured synthesis model identifier +4. **Connectivity status** — whether the synthesis endpoint is + reachable (Ollama only) +5. **Model/credential availability** — whether the model is + available (Ollama) or config is complete (Vertex) + +#### Scenario: Ollama synthesis provider configured and reachable + +- **GIVEN** a vault with synthesis configured as provider `ollama` + with model `llama3.2:3b` and the Ollama instance is running +- **WHEN** the user runs `dewey doctor` +- **THEN** the output MUST include a "Synthesis Layer" section header + with the model and endpoint +- **AND** the connectivity check MUST report PASS with "running + (external)" +- **AND** the model availability check MUST report PASS with the + model name and "ready" + +#### Scenario: Ollama synthesis provider configured but unreachable + +- **GIVEN** a vault with synthesis configured as provider `ollama` + but the Ollama instance is not running +- **WHEN** the user runs `dewey doctor` +- **THEN** the connectivity check MUST report WARN with "not running" + or PASS with "not installed (optional)" +- **AND** the model availability check MUST report WARN with "skipped + (ollama not reachable)" + +#### Scenario: Vertex synthesis provider configured + +- **GIVEN** a vault with synthesis configured as provider `vertex` + with project, region, and model set +- **WHEN** the user runs `dewey doctor` +- **THEN** the output MUST include a "Synthesis Layer" section header + with the model and provider type +- **AND** the provider check MUST report PASS with "vertex" and the + project/region +- **AND** doctor MUST NOT make live API calls to Vertex AI + +#### Scenario: Vertex synthesis provider misconfigured + +- **GIVEN** a vault with synthesis configured as provider `vertex` + but missing required fields (project or region) +- **WHEN** the user runs `dewey doctor` +- **THEN** the configuration check MUST report FAIL with a message + identifying the specific missing field (e.g., "project" or "region") + +#### Scenario: Dual-endpoint setup shows independent probes + +- **GIVEN** a vault with embedding endpoint `http://emb:11434` and + synthesis configured as provider `ollama` with endpoint + `http://synth:11434` +- **WHEN** the user runs `dewey doctor` +- **THEN** the Embedding Layer section MUST show `http://emb:11434` +- **AND** the Synthesis Layer section MUST show `http://synth:11434` +- **AND** the connectivity checks MUST probe each endpoint independently + +#### Scenario: No synthesis provider configured + +- **GIVEN** a vault with no synthesis configuration (no config.yaml + synthesis section, no env vars) +- **WHEN** the user runs `dewey doctor` +- **THEN** the output MUST include a "Synthesis Layer" section +- **AND** the section MUST report PASS with "not configured + (optional)" status +- **AND** doctor MUST NOT report this as a failure or warning + +### Requirement: Token safety in diagnostic output + +The `dewey doctor` synthesis section MUST NOT display OAuth tokens, +API keys, or credentials in its output. Only configuration metadata +(endpoint URLs, project IDs, region names, model names) MAY be +displayed. + +#### Scenario: Vertex provider output contains no credentials + +- **GIVEN** a vault with synthesis configured as provider `vertex` +- **WHEN** the user runs `dewey doctor` +- **THEN** the output MUST include project ID and region +- **AND** the output MUST NOT include any OAuth token, bearer token, + or API key values + +## MODIFIED Requirements + +_None._ + +## REMOVED Requirements + +_None._ + diff --git a/openspec/changes/doctor-synthesis-diagnostics/tasks.md b/openspec/changes/doctor-synthesis-diagnostics/tasks.md new file mode 100644 index 0000000..3038bf3 --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/tasks.md @@ -0,0 +1,66 @@ + + +## 1. Implement Synthesis Layer section in dewey doctor + +- [x] 1.1 Add Synthesis Layer section to `runDoctorChecks()` in `cli.go` + - Read synthesis config via `llm.ReadSynthesisConfig(deweyDir)` + - Insert new section between Embedding Layer (line 1518) and MCP Server (line 1520) + - Handle three provider cases: ollama, vertex, unconfigured + - **Ollama path**: section header with model+endpoint, `ensureOllama(synthEndpoint, false, nil)` for connectivity, `OllamaSynthesizer.Available()` for model check + - **Vertex path**: section header with model+provider, report config completeness (project, region, model), no live API calls + - **Unconfigured path**: report PASS "not configured (optional)" + - Files: `cli.go` + +## 2. Add tests for Synthesis Layer diagnostics + +- [x] 2.1 [P] Add test for unconfigured synthesis provider + - Verify "Synthesis Layer" section appears in doctor output + - Verify "not configured" message with PASS status + - Use `t.TempDir()` vault with no config.yaml synthesis section + - Files: `cli_test.go` + +- [x] 2.2 [P] Add test for Ollama synthesis provider + - Configure synthesis with ollama provider in config.yaml + - Use `newMockOllamaServer()` for connectivity check (accessible from `main_test.go`, same package) + - Verify section header includes model and endpoint + - Verify connectivity and model availability checks + - Verify synthesis endpoint is independent from embedding endpoint (dual-endpoint scenario) + - Files: `main_test.go` (colocated with `newMockOllamaServer` helper) + +- [x] 2.3 [P] Add test for Vertex synthesis provider + - Configure synthesis with vertex provider in config.yaml + - Verify section header includes model and "vertex" provider type + - Verify config completeness check (project + region present) + - Verify Vertex provider reports config status without requiring GCP credentials (test passes without application-default credentials) + - Files: `cli_test.go` + +- [x] 2.4 [P] Add test for Vertex synthesis provider misconfigured + - Configure synthesis with vertex provider but missing project/region + - Verify FAIL check for missing required fields + - Files: `cli_test.go` +- [x] 2.5 [P] Add test for Ollama synthesis provider unreachable + - Configure synthesis with Ollama provider pointing to closed endpoint + - Verify model check is skipped with "ollama not reachable" message + - Files: `main_test.go` (uses `newMockOllamaServer` from same package) +- [x] 2.6 [P] Add test for unknown synthesis provider + - Configure synthesis with unsupported provider name + - Verify FAIL check reporting the unknown provider + - Files: `cli_test.go` + +## 3. Verification + +- [x] 3.1 Run CI-equivalent checks (`go build ./...`, `go vet ./...`, `go test -race -count=1 ./...`) and verify `runDoctorChecks` CRAPload stays within CI thresholds (`--max-crapload=48`) +- [x] 3.2 Verify constitution alignment: Composability First (graceful degradation when unconfigured), Observable Quality (full provider stack diagnostics), Testability (no external services in tests) +- [x] 3.3 Verify documentation impact (AGENTS.md doctor section, GoDoc comments) + +