From fbd5000346fefb1652fedf7ecd702ecdefa231ac Mon Sep 17 00:00:00 2001 From: Yvonne Devlin Date: Thu, 20 Aug 2026 10:40:34 +0000 Subject: [PATCH 1/2] docs: add triage and openspec artifacts for issue #102 (doctor synthesis diagnostics) --- .triage/102/triage.md | 88 +++++++++++++ .../.openspec.yaml | 2 + .../doctor-synthesis-diagnostics/design.md | 120 ++++++++++++++++++ .../doctor-synthesis-diagnostics/proposal.md | 79 ++++++++++++ .../specs/doctor-synthesis-section.md | 93 ++++++++++++++ .../doctor-synthesis-diagnostics/tasks.md | 56 ++++++++ 6 files changed, 438 insertions(+) create mode 100644 .triage/102/triage.md create mode 100644 openspec/changes/doctor-synthesis-diagnostics/.openspec.yaml create mode 100644 openspec/changes/doctor-synthesis-diagnostics/design.md create mode 100644 openspec/changes/doctor-synthesis-diagnostics/proposal.md create mode 100644 openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md create mode 100644 openspec/changes/doctor-synthesis-diagnostics/tasks.md 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/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..4366211 --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/design.md @@ -0,0 +1,120 @@ +## 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. +- **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 isn't needed here — +tests use mock HTTP servers and environment variables, not injected +dependencies. + +### 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). + 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..d5297e5 --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md @@ -0,0 +1,93 @@ +## 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 indicating which + required field is missing + +#### 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..27603d8 --- /dev/null +++ b/openspec/changes/doctor-synthesis-diagnostics/tasks.md @@ -0,0 +1,56 @@ + + +## 1. Implement Synthesis Layer section in dewey doctor + +- [ ] 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 + +- [ ] 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` + +- [ ] 2.2 [P] Add test for Ollama synthesis provider + - Configure synthesis with ollama provider in config.yaml + - Use `newMockOllamaServer()` for connectivity check + - Verify section header includes model and endpoint + - Verify connectivity and model availability checks + - Files: `main_test.go` + +- [ ] 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 no live API calls are made + - Files: `cli_test.go` + +- [ ] 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` + +## 3. Verification + +- [ ] 3.1 Run CI-equivalent checks (`go build ./...`, `go vet ./...`, `go test -race -count=1 ./...`) +- [ ] 3.2 Verify constitution alignment: Composability First (graceful degradation when unconfigured), Observable Quality (full provider stack diagnostics), Testability (no external services in tests) +- [ ] 3.3 Verify documentation impact (AGENTS.md doctor section, GoDoc comments) + From 41d3846f8a11535e88eaf8dee275867be1de5238 Mon Sep 17 00:00:00 2001 From: Yvonne Devlin Date: Thu, 20 Aug 2026 12:47:14 +0000 Subject: [PATCH 2/2] feat: add synthesis layer diagnostics to dewey doctor Add a Synthesis Layer section to dewey doctor output between the existing Embedding Layer and MCP Server sections. Reports provider type, resolved endpoint, model, connectivity, and model availability for all three provider states (Ollama, Vertex, unconfigured). - Ollama: connectivity via ensureOllama + model availability check - Vertex: config completeness validation (no live API calls) - Unconfigured: PASS with 'not configured (optional)' Tests cover all provider states including Ollama unreachable, dual-endpoint independence, unknown provider, Vertex misconfigured, and token safety negative assertions. Closes #102 --- README.md | 2 +- cli.go | 64 +++++++ cli_test.go | 170 ++++++++++++++++++ main_test.go | 94 ++++++++++ .../doctor-synthesis-diagnostics/design.md | 16 +- .../specs/doctor-synthesis-section.md | 14 +- .../doctor-synthesis-diagnostics/tasks.md | 32 ++-- 7 files changed, 374 insertions(+), 18 deletions(-) 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/design.md b/openspec/changes/doctor-synthesis-diagnostics/design.md index 4366211..2f5b5ef 100644 --- a/openspec/changes/doctor-synthesis-diagnostics/design.md +++ b/openspec/changes/doctor-synthesis-diagnostics/design.md @@ -57,7 +57,10 @@ cognitive load. - **Ollama provider**: Reuse `ensureOllama(synthEndpoint, false, nil)` for connectivity, then `OllamaSynthesizer.Available()` for model - availability. This is identical to the embedding pattern. + 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 @@ -83,9 +86,11 @@ Like the embedding section which constructs inline. No new parameters to `runDoctorChecks()`. **Rationale**: Consistency with existing pattern. The `NoopSynthesizer` -test double is available for integration tests but isn't needed here — -tests use mock HTTP servers and environment variables, not injected -dependencies. +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 @@ -117,4 +122,7 @@ 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/specs/doctor-synthesis-section.md b/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md index d5297e5..dc469d5 100644 --- a/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md +++ b/openspec/changes/doctor-synthesis-diagnostics/specs/doctor-synthesis-section.md @@ -55,8 +55,18 @@ The section MUST report: - **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 indicating which - required field is missing +- **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 diff --git a/openspec/changes/doctor-synthesis-diagnostics/tasks.md b/openspec/changes/doctor-synthesis-diagnostics/tasks.md index 27603d8..3038bf3 100644 --- a/openspec/changes/doctor-synthesis-diagnostics/tasks.md +++ b/openspec/changes/doctor-synthesis-diagnostics/tasks.md @@ -12,7 +12,7 @@ ## 1. Implement Synthesis Layer section in dewey doctor -- [ ] 1.1 Add Synthesis Layer section to `runDoctorChecks()` in `cli.go` +- [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 @@ -23,34 +23,44 @@ ## 2. Add tests for Synthesis Layer diagnostics -- [ ] 2.1 [P] Add test for unconfigured synthesis provider +- [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` -- [ ] 2.2 [P] Add test for Ollama synthesis provider +- [x] 2.2 [P] Add test for Ollama synthesis provider - Configure synthesis with ollama provider in config.yaml - - Use `newMockOllamaServer()` for connectivity check + - 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 - - Files: `main_test.go` + - Verify synthesis endpoint is independent from embedding endpoint (dual-endpoint scenario) + - Files: `main_test.go` (colocated with `newMockOllamaServer` helper) -- [ ] 2.3 [P] Add test for Vertex synthesis provider +- [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 no live API calls are made + - Verify Vertex provider reports config status without requiring GCP credentials (test passes without application-default credentials) - Files: `cli_test.go` -- [ ] 2.4 [P] Add test for Vertex synthesis provider misconfigured +- [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 -- [ ] 3.1 Run CI-equivalent checks (`go build ./...`, `go vet ./...`, `go test -race -count=1 ./...`) -- [ ] 3.2 Verify constitution alignment: Composability First (graceful degradation when unconfigured), Observable Quality (full provider stack diagnostics), Testability (no external services in tests) -- [ ] 3.3 Verify documentation impact (AGENTS.md doctor section, GoDoc comments) +- [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) +