Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions .triage/102/triage.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
64 changes: 64 additions & 0 deletions cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
170 changes: 170 additions & 0 deletions cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
Loading
Loading