Skip to content

[module-hygiene-insights-api-transport] Reuse aiProvider for Insights LLM calls #5617

Description

@atomantic

Slice/date: Server-side AI provider/runner boundary, specifically server/services/insightsService.js and its service/test boundary; audited 2026-09-01.

Problem

server/services/insightsService.js:166-226 defines and exports a second callProviderAISimple; its own JSDoc says it “Replicate[s] callProviderAISimple pattern from taste-questionnaire.js.” The existing owner is server/services/aiProvider.js:136-195 (the HTTP completion) and server/services/aiProvider.js:221-336 (provider readiness, status, timeout, model recovery, response classification, and the public helper). Insights invokes its copy at server/services/insightsService.js:430-433 and :537-540.

This is already behaviorally divergent, not just duplicate naming. The shared service rejects a missing/whitespace completion at server/services/aiProvider.js:172-185, while the Insights copy accepts a valid JSON response with empty content and returns { text: '' } at server/services/insightsService.js:212-222. The cross-domain path then writes that empty result to narrative.json at server/services/insightsService.js:546-556. The two test suites consequently maintain separate transport contracts: server/services/insightsService.test.js:21-98 tests the copy, while server/services/aiProvider.test.js:86-151 tests the shared implementation.

History shows the maintenance cost: d0a8ebf40 promoted aiProvider into server/services while this copy remained, and 015e617be plus f712182c2 required an Insights-specific non-JSON fix and test. Prior issue #2037 verified that Insights generation is user-triggered; this finding is about the duplicated transport after that trigger contract, not about adding an unsolicited call.

Impact

A provider response can be classified as a successful empty completion in Insights while the same response is a provider error everywhere using aiProvider. Shared fixes for status reporting, provider readiness/model recovery, timeout behavior, or malformed/empty response handling do not automatically reach the two user-triggered Insights generation workflows. The result is different runtime/data behavior and repeated manual transport patches; the cross-domain workflow can persist an empty narrative instead of surfacing the provider failure.

Reuse search

Searched server/lib/README.md, server/lib/index.js, server/lib/aiToolkit/index.js, and repository imports of callProviderAISimple. The existing server/services/aiProvider.js is the appropriate owner; no new helper or catalog entry is needed. Prior layering work in #4901/PR #5029 deliberately moved provider orchestration into server/services and left only pure text helpers in server/lib.

Fix

Make server/services/aiProvider.js the sole owner of the simple API completion transport. Delete the implementation and transport-only imports/constants from server/services/insightsService.js, and import callProviderAISimple from ./aiProvider.js. Keep Insights-specific context assembly, JSON/text parsing, cached reads, and persistence in insightsService.js; only the provider call crosses the shared seam. Move the non-JSON/empty/HTTP transport matrix into server/services/aiProvider.test.js, and keep server/services/insightsService.test.js focused on the public Insights workflows and disk-only read paths.

Compatibility obligations: preserve the exported Insights functions, their return shapes, prompt/model/temperature choices, themes.json and narrative.json paths, and the no-cold-bootstrap read behavior. Update the expected test for the intentional canonical classification of an empty completion; do not change the Insights trigger contract.

Acceptance criteria

  • server/services/insightsService.js contains no second callProviderAISimple implementation and no unused transport imports/constants.
  • Both generateThemeAnalysis and refreshCrossDomainNarrative use the shared server/services/aiProvider.js helper.
  • A malformed, non-JSON, non-2xx, or whitespace-only provider response reaches the shared error classification and does not write a successful empty narrative/theme result.
  • Insights cached-read entry points remain disk-only and make no provider call.
  • server/services/aiProvider.test.js covers the transport classification, and server/services/insightsService.test.js covers the public workflow contract without re-testing the deleted transport copy.

Scope: medium

Metadata

Metadata

Assignees

Labels

code-qualityProposed from a module-hygiene auditeffort:mediumEffort: mediummodel:lightModel size: lightplanTracked by /do:replanplanner:gpt-5-6-lunaPlan authored by the gpt-5-6-luna model

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions