Skip to content

fix(nemo-insights): export analyst result types from package __init__ and update prompt import path - #1220

Draft
mdcox wants to merge 1 commit into
mainfrom
fix-analyst-new-insight-import/morganc
Draft

fix(nemo-insights): export analyst result types from package __init__ and update prompt import path#1220
mdcox wants to merge 1 commit into
mainfrom
fix-analyst-new-insight-import/morganc

Conversation

@mdcox

@mdcox mdcox commented Aug 10, 2026

Copy link
Copy Markdown

Summary

The analyst LLM was crashing with AttributeError: module 'nemo_insights_plugin.analyst.functions.insights' has no attribute 'NewInsight'. The system prompt told the model to call return_result(result=AnalystResult(...)) but gave no import path. The model guessed NewInsight lived in the insights module (the last insights-related module it had used), which was wrong — it lives in analyst.result.

Related Issue

Changes

  • Add plugins/nemo-insights/src/nemo_insights_plugin/analyst/__init__.py: exports AnalystResult, NewInsight, and InsightUpdate as the stable public API of the analyst package, decoupling consumers from internal module layout.
  • Update the analyst system prompt in agent.py to show the canonical import path (from nemo_insights_plugin.analyst import ...) so the model no longer has to guess.

Type of Change

  • Code change (feature, bug fix, or refactor)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification: 146 non-testbed nemo-insights tests pass; the changed modules are exercised by the existing analyst test suite.
  • Tests not applicable — justification:
  • Documentation updated for user-visible behavior — the system prompt is the user-facing documentation for the model.
  • Documentation not applicable — justification:

Verification

  • Pull request title follows the repository's Conventional Commit format
  • Every commit includes an appropriate Signed-off-by: trailer (horde@nvidia.com)
  • uv run pre-commit run -a passes, or any blocked checks are identified below
  • Targeted tests pass, or tests are marked not applicable above
  • No secrets, API keys, or credentials are included

Targeted validation:

# Import sanity check
python -c "from nemo_insights_plugin.analyst import AnalystResult, NewInsight, InsightUpdate; print('ok')"
# -> ok

# Non-testbed plugin tests (testbed tests require external services and fail pre-existing)
uv run --frozen pytest plugins/nemo-insights/ --ignore=plugins/nemo-insights/tests/testbed -v
# -> 146 passed, 1 skipped

# Pre-commit (all hooks pass; uv-lock hook reports version mismatch 0.9.30 vs required 0.9.14 — unrelated to this change, uv-lock-check confirms lock is in sync)
uv run pre-commit run -a

… and update prompt import path

The analyst LLM was generating execute_python code that imported NewInsight
from nemo_insights_plugin.analyst.functions.insights, which does not define
it. Root cause: the system prompt told the model to call return_result with
an AnalystResult but gave no import path, so the model guessed based on the
last insights-related module it knew about.

Fix by:
- Adding a public API to nemo_insights_plugin.analyst.__init__ that re-exports
  AnalystResult, NewInsight, and InsightUpdate from analyst.result. This gives
  consumers a stable import path that survives internal module reorganization.
- Updating the analyst system prompt to show the canonical import explicitly,
  eliminating the guesswork.

Signed-off-by: Horde <horde@nvidia.com>
@github-actions github-actions Bot added the fix label Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 32002/40616 78.8% 63.6%
Integration Tests 18560/38542 48.2% 20.8%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant