refactor: rename Sarthi→TrackGuard, Charaka→AnomalyDetector, Mantripa…#25
Conversation
…rishad→AlertCouncil, Nyayadish→ConflictResolver, SarthiRouter→WorkflowRouter - Product name: Sarthi → TrackGuard (all configs, prompts, strings) - Go: SarthiRouter → WorkflowRouter, sarthi_router.go → workflow_router.go - Python: Charaka → AnomalyDetector, CharakaAlert → AnomalyAlert - Python: Mantriparishad → AlertCouncil, CouncilAlert → AlertCouncilResult - Python: Nyayadish → ConflictResolver - Python: get_sarthi_database_url → get_database_url - Files: sarthi_scheduler.py → trackguard_scheduler.py - Files: sarthi-mocks.json → trackguard-mocks.json - All config strings sarthi.* → trackguard.* - README: updated with TrackGuard naming, removed Kautilya/Arthashastra refs - .gitignore: added graphify-out/
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (130)
📝 WalkthroughWalkthroughThis PR adds an Opencode Graphify plugin, refactors AI LLM and database configuration, renames several exported AI types, switches many runtime namespaces from ChangesTrackGuard platform transition
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request completes the rebranding of the project from 'Sarthi' to 'TrackGuard' across the entire codebase, including documentation, configuration files, and test suites. It also refactors the LLM client infrastructure in 'apps/ai/src/config/llm.py' to use the OpenAI SDK, introducing support for streaming completions. Additionally, the PR adds a new OpenCode plugin for knowledge graph reminders and updates various infrastructure configurations, such as Kafka topics and database connection strings. The review identified several critical issues, including a task queue name mismatch between the Go service and Python worker, an incorrect implementation of async streaming, potential resource leaks in the client reset logic, and suggestions for improving test environment isolation and documentation consistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
|
|
||
| _, err := h.temporalClient.StartWorkflow(c.Context(), workflowID, "sarthi-queue", input) | ||
| _, err := h.temporalClient.StartWorkflow(c.Context(), workflowID, "trackguard-queue", input) |
There was a problem hiding this comment.
There is an inconsistency in the Temporal task queue name. This Go code hardcodes the queue name as "trackguard-queue", while the Python worker is configured to listen on "TRACKGUARD-MAIN-QUEUE" (from apps/ai/src/worker.py). This mismatch will prevent the Python worker from picking up workflows started by this Go service. These names must be identical. It would also be best practice to make the queue name configurable rather than hardcoding it.
| _, err := h.temporalClient.StartWorkflow(c.Context(), workflowID, "trackguard-queue", input) | |
| _, err := h.temporalClient.StartWorkflow(c.Context(), workflowID, "TRACKGUARD-MAIN-QUEUE", input) |
| async def chat_completion_stream_async( | ||
| messages: list[dict[str, str]], | ||
| model: str | None = None, | ||
| max_tokens: int = 500, | ||
| temperature: float = 0.0, | ||
| json_mode: bool = False, | ||
| **extra: Any, | ||
| ) -> AsyncGenerator[str, None]: | ||
| """Async stream chat completion chunks as they arrive. | ||
|
|
||
| Yields content delta strings. Use for async UI updates or | ||
| token-by-token processing in async contexts. | ||
|
|
||
| Usage: | ||
| async for chunk in chat_completion_stream_async(messages=[...]): | ||
| print(chunk, end="", flush=True) | ||
| """ | ||
| global _async_client | ||
| if _async_client is None: | ||
| base_url, api_key = _provider_config() | ||
| _async_client = AsyncOpenAI( | ||
| base_url=base_url, | ||
| api_key=api_key, | ||
| timeout=60.0, | ||
| ) | ||
| kwargs = _build_kwargs(messages, model, max_tokens, temperature, json_mode, **extra) | ||
| async with _async_client.chat.completions.stream(**kwargs) as stream: | ||
| async for event in stream: | ||
| if event.type == "content.delta" and event.delta: | ||
| yield event.delta |
There was a problem hiding this comment.
The implementation of chat_completion_stream_async appears incorrect for the openai Python SDK. The use of _async_client.chat.completions.stream() and iterating over event objects with a .type attribute does not match the openai library's API for streaming. This will likely cause a runtime error.
The implementation should be similar to the synchronous chat_completion_stream function, using await _async_client.chat.completions.create(stream=True) and iterating through the resulting ChatCompletionChunk objects.
async def chat_completion_stream_async(
messages: list[dict[str, str]],
model: str | None = None,
max_tokens: int = 500,
temperature: float = 0.0,
json_mode: bool = False,
**extra: Any,
) -> AsyncGenerator[str, None]:
"""Async stream chat completion chunks as they arrive.
Yields content delta strings. Use for async UI updates or
token-by-token processing in async contexts.
Usage:
async for chunk in chat_completion_stream_async(messages=[...]):
print(chunk, end="", flush=True)
"""
global _async_client
if _async_client is None:
base_url, api_key = _provider_config()
_async_client = AsyncOpenAI(
base_url=base_url,
api_key=api_key,
timeout=60.0,
)
kwargs = _build_kwargs(messages, model, max_tokens, temperature, json_mode, **extra)
stream = await _async_client.chat.completions.create(**kwargs, stream=True)
async for chunk in stream:
if chunk.choices and chunk.choices[0].delta.content:
yield chunk.choices[0].delta.content| output.args.command = | ||
| 'echo "[graphify] Knowledge graph available. Read graphify-out/GRAPH_REPORT.md for god nodes and architecture context before searching files." && ' + | ||
| output.args.command; |
There was a problem hiding this comment.
The string concatenation for building the command can be error-prone and hard to read. Using a template literal would make the code cleaner and safer, especially if more complex commands are built in the future.
| output.args.command = | |
| 'echo "[graphify] Knowledge graph available. Read graphify-out/GRAPH_REPORT.md for god nodes and architecture context before searching files." && ' + | |
| output.args.command; | |
| output.args.command = `echo "[graphify] Knowledge graph available. Read graphify-out/GRAPH_REPORT.md for god nodes and architecture context before searching files." && ${output.args.command}`; |
| DEEPEVAL_ID=73dfc665-6e63-423d-9844-b5787b1991d8 | ||
| DEEPEVAL_STATUS=old | ||
| DEEPEVAL_LAST_FEATURE=evaluation | ||
| DEEPEVAL_EVALUATION_STATUS=old |
There was a problem hiding this comment.
This telemetry file appears to be auto-generated by a tool and likely shouldn't be checked into source control. Consider removing this file and adding the .deepeval/ directory to your .gitignore file to prevent committing such files in the future.
| def reset_client() -> None: | ||
| """Reset cached client.""" | ||
| global _client | ||
| """Reset cached clients.""" | ||
| global _client, _async_client | ||
| if _client: | ||
| _client.close() | ||
| _client = None | ||
| _async_client = None |
There was a problem hiding this comment.
The reset_client function doesn't properly close the _async_client, which can lead to resource leaks. The AsyncOpenAI client's close() method is a coroutine and must be awaited. Since this function is synchronous, you can use asyncio.run() to execute the async close operation.
| def reset_client() -> None: | |
| """Reset cached client.""" | |
| global _client | |
| """Reset cached clients.""" | |
| global _client, _async_client | |
| if _client: | |
| _client.close() | |
| _client = None | |
| _async_client = None | |
| def reset_client() -> None: | |
| """Reset cached clients.""" | |
| import asyncio | |
| global _client, _async_client | |
| if _client: | |
| _client.close() | |
| if _async_client: | |
| asyncio.run(_async_client.close()) | |
| _client = None | |
| _async_client = None |
| def _set_env(): | ||
| """Set environment variables for tests.""" | ||
| """Set environment variables for tests, restoring originals on teardown.""" | ||
| originals = { | ||
| "GROQ_API_KEY": os.environ.get("GROQ_API_KEY"), | ||
| "GROQ_CHAT_MODEL": os.environ.get("GROQ_CHAT_MODEL"), | ||
| "OPENROUTER_EMBED_MODEL": os.environ.get("OPENROUTER_EMBED_MODEL"), | ||
| } | ||
| os.environ.setdefault("GROQ_API_KEY", "gsk_test_key") | ||
| os.environ.setdefault("GROQ_CHAT_MODEL", "qwen/qwen3-32b") | ||
| os.environ.setdefault("OPENROUTER_EMBED_MODEL", "nvidia/llama-nemotron-embed-vl-1b-v2:free") | ||
| yield | ||
| for key, value in originals.items(): | ||
| if value is None: | ||
| os.environ.pop(key, None) | ||
| else: | ||
| os.environ[key] = value |
There was a problem hiding this comment.
The _set_env fixture modifies global state (os.environ) which can affect other tests if not properly isolated. While the teardown logic is a good step, a more robust approach is to use monkeypatch.setenv which is managed by pytest and ensures proper cleanup without manual teardown code.
@pytest.fixture(autouse=True)
def _set_env(monkeypatch):
"""Set environment variables for tests."""
monkeypatch.setenv("GROQ_API_KEY", "gsk_test_key")
monkeypatch.setenv("GROQ_CHAT_MODEL", "qwen/qwen3-32b")
monkeypatch.setenv("OPENROUTER_EMBED_MODEL", "nvidia/llama-nemotron-embed-vl-1b-v2:free")| - Self-hosted: `http://langfuse.local` | ||
| - Cloud: `https://cloud.langfuse.com` | ||
| 2. Filter by: | ||
| - **Trace name:** `sarthi-agent-run` |
There was a problem hiding this comment.
The trace name sarthi-agent-run seems to be a leftover from before the refactoring. For consistency with the project's renaming to TrackGuard, this should probably be trackguard-agent-run.
| - **Trace name:** `sarthi-agent-run` | |
| - **Trace name:** `trackguard-agent-run` |
1 participant
…rishad→AlertCouncil, Nyayadish→ConflictResolver, SarthiRouter→WorkflowRouter
Summary by CodeRabbit
Release Notes
Infrastructure
Branding & Identity
Configuration
Documentation