{ "batch": "Cross-cutting Sweep", "batch_index": 5, "assessments": { "error_consistency": 68.0 }, "dimension_notes": { "error_consistency": { "evidence": [ "RPC paths mix strict and lax decode behavior: `cmd/serve.go` returns decode errors for `devour_scrape`/`devour_ask` but ignores decode errors for `devour_query` and `devour_sync` (`_ = json.Unmarshal(...)`).", "`cmd/serve.go` `devour_status` ignores errors from `LoadSourceState` and `EnsureIndexed` then dereferences `idxStats.Documents`, which can panic on nil when indexing fails.", "`internal/server/server.go` exposes raw internal errors to clients (`Message: err.Error()`), while parse/invalid-request errors are normalized strings; HTTP transport always emits 400 for any RPC error, collapsing error classes.", "`internal/ai/openai.go` does not check HTTP status before JSON decode, unlike scraper fetchers in `internal/scraper/external/*.go` and `internal/scraper/openapi.go` that explicitly gate on status codes.", "`internal/search/engine.go` and `internal/scraper/openapi.go` mix wrapped and passthrough errors in adjacent paths, producing uneven context for operators." ], "impact_scope": "subsystem", "fix_scope": "multi_file_refactor", "confidence": "high" } }, "findings": [ { "dimension": "error_consistency", "identifier": "rpc_decode_and_response_contract_drift", "summary": "RPC methods use inconsistent decode and error-response contracts", "related_files": [ "cmd/serve.go", "internal/server/server.go" ], "evidence": [ "`cmd/serve.go` ignores JSON decode errors in `devour_query`/`devour_sync` but returns decode errors in `devour_scrape`/`devour_ask`.", "`internal/server/server.go` returns raw `err.Error()` in RPC payloads and maps all RPC errors to HTTP 400 in `writeRPC`." ], "suggestion": "Define one RPC error policy: always validate/decode params the same way, map known classes to stable RPC codes/messages, and map transport HTTP statuses by error class (client input vs internal failure).", "confidence": "high", "impact_scope": "subsystem", "fix_scope": "multi_file_refactor" }, { "dimension": "error_consistency", "identifier": "serve_status_swallows_errors_then_dereferences_nil", "summary": "Status handler ignores errors and can panic from nil stats", "related_files": [ "cmd/serve.go", "internal/search/engine.go" ], "evidence": [ "`cmd/serve.go` uses `state, _ := projectstate.LoadSourceState(...)` and `idxStats, _ := engine.EnsureIndexed(ctx)` in `devour_status`.", "The same block unconditionally reads `idxStats.Documents` and `idxStats.LastIndexedAt`, which is unsafe if `EnsureIndexed` returned an error." ], "suggestion": "Handle both errors explicitly in `devour_status`; return structured partial-status with an error field or fail the method uniformly, but do not ignore and dereference.", "confidence": "high", "impact_scope": "module", "fix_scope": "single_edit" }, { "dimension": "error_consistency", "identifier": "openai_http_error_path_not_normalized", "summary": "OpenAI client lacks explicit HTTP status error handling", "related_files": [ "internal/ai/openai.go", "internal/scraper/openapi.go", "internal/scraper/external/astrodocs.go" ], "evidence": [ "`internal/ai/openai.go` decodes response bodies directly and only checks `embeddingResp.Error`/`chatResp.Error`; non-2xx responses without expected JSON become generic decode errors.", "`internal/scraper/openapi.go` and external scrapers explicitly validate HTTP status before body parsing and return explicit HTTP status errors." ], "suggestion": "Add explicit non-2xx handling in both OpenAI request paths: include status code, bounded response body excerpt, and endpoint context before JSON decode.", "confidence": "high", "impact_scope": "module", "fix_scope": "multi_file_refactor" }, { "dimension": "error_consistency", "identifier": "mixed_wrapping_vs_passthrough_in_core_flows", "summary": "Adjacent paths alternate between wrapped and raw error returns", "related_files": [ "internal/search/engine.go", "internal/scraper/openapi.go", "internal/config/config.go" ], "evidence": [ "`internal/search/engine.go` frequently returns raw errors (`return nil, err`) from filesystem/index operations.", "`internal/scraper/openapi.go` similarly passes through request/file errors raw in `readSpec`, while other branches wrap with operation context.", "`internal/config/config.go` demonstrates contextual wrapping style (`read config`, `parse config`), creating drift against less-informative paths." ], "suggestion": "Adopt a package-level rule: wrap external boundary failures with operation context (read/write/parse/network) and preserve `%w`; apply consistently across search/openapi paths.", "confidence": "medium", "impact_scope": "subsystem", "fix_scope": "multi_file_refactor" }, { "dimension": "error_consistency", "identifier": "scanner_pipeline_fail_open_is_inconsistent", "summary": "Quality scan path mixes fail-open and fail-fast behaviors", "related_files": [ "internal/quality/scanner.go", "internal/quality/plugins/go/analyzers/test_coverage.go", "internal/quality/plugins/go/analyzers/detectors.go" ], "evidence": [ "`internal/quality/scanner.go` logs detector failures and continues, silently reducing coverage of reported findings.", "`test_coverage.go` returns `(nil, nil)` for missing `go` tool or missing generated coverage file, while other detector failures return explicit errors.", "`detectors.go` has parse/count paths that silently `continue` on per-file errors, further mixing behavior." ], "suggestion": "Standardize detector error semantics with typed outcomes (hard error, soft-skip with reason, per-file skip count) and surface these in scan results so degraded scans are explicit.", "confidence": "medium", "impact_scope": "subsystem", "fix_scope": "architectural_change" } ] }