SDK violates project rule: direct `os.getenv` in search tools and monitor

Open
#3,458 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
72/100
Issue type
Refactor
Clarity
Mostly clear
Activity status
Quiet
Tech stack
python
Domain
api, backend

Research direction

Start with CLAUDE.md and the five listed SDK files, locating each environment read and its constructor. Then trace the corresponding backend/services/.../*_service.py callers to understand how configuration reaches the SDK. Done means SDK modules no longer read these variables directly, callers pass configuration explicitly, and relevant tests cover per-instance configuration without environment mutation.

Written by the indexing model from the issue text.

Description

Critical Improvement

CLAUDE.md is explicit:

SDK modules (sdk/) should never read environment variables directly - accept configuration via parameters

But the SDK currently has five direct env reads:

File Line Variable
sdk/nexent/core/tools/linkup_search_tool.py 65 DATA_PROCESS_SERVICE
sdk/nexent/core/tools/tavily_search_tool.py 68 DATA_PROCESS_SERVICE
sdk/nexent/core/tools/exa_search_tool.py 69 DATA_PROCESS_SERVICE
sdk/nexent/monitor/monitoring.py 1046 ENABLE_MODEL_MONITORING
sdk/nexent/monitor/monitoring.py 1048-1051 MODEL_MONITORING_BATCH_SIZE, MODEL_MONITORING_FLUSH_INTERVAL_SECONDS

For example, linkup_search_tool.py:

def __init__(
    self,
    linkup_api_key: str = Field(description="Linkup API key"),
    observer: MessageObserver = Field(...),
    max_results: int = Field(..., default=3),
    image_filter: bool = Field(..., default=True)
):
    super().__init__()
    ...
    self.data_process_service = os.getenv("DATA_PROCESS_SERVICE")  # <-- violates SDK rule

The tavily and exa variants even carry a # TODO add data_process_service comment next to the line, which suggests the author already knew this was a stop-gap.

Why this matters
  1. The SDK is imported by backend/ and shipped as pip install nexent. External consumers will not have DATA_PROCESS_SERVICE exported and will silently get None, breaking the image-filter callback path inside forward().
  2. The two monitoring knobs in MonitoringRecordBuffer.__init__ mean an SDK user must set process-wide env vars to tune behaviour — there is no per-instance override.
  3. Tests can't override these without monkeypatch.setenv, which adds shared global state to the test suite.
Suggested fix

Promote each variable to a constructor parameter with a default of None, and have backend/services/.../*_service.py read from consts.const and pass the value through, per the documented architecture rule.

Severity: Medium (architectural debt; functionally works for the in-tree integration but blocks redistribution and clean tests).

Dominant language
Python
Stars
5.9k
Forks
731
Avg merge
19h 34m
Merged PRs (30d)
172

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ModelEngine-Group/nexent

All issues in ModelEngine-Group/nexent

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.