feat(HYBIM-730): rename Log Streams → Agent Streams, Metrics → Evaluators#100
feat(HYBIM-730): rename Log Streams → Agent Streams, Metrics → Evaluators#100adityamehra wants to merge 3 commits into
Conversation
…tors Introduces the new canonical domain entity names required by HYBIM-730: - **AgentStream** replaces LogStream (new class in agent_stream.py that subclasses LogStream; returns AgentStream from Project methods) - **AgentStreams** replaces LogStreams service class (agent_streams.py); module-level helpers: get_agent_stream, list_agent_streams, create_agent_stream, enable_evaluators - **Evaluator** replaces Metric base class (evaluator.py) - **LlmEvaluator**, **CodeEvaluator**, **LocalEvaluator**, **SplunkAOEvaluator** replace their Metric-prefixed counterparts - **Evaluators** replaces Metrics service class (evaluators.py); helpers: create_custom_llm_evaluator, get_evaluators, delete_evaluator Backward compatibility: - Old names (LogStream, Metric, LlmMetric, …) are preserved via PEP 562 __getattr__ in __init__.py and emit DeprecationWarning - Project.create_log_stream / list_log_streams / logstreams delegate to the new methods with DeprecationWarning - __future__/agent_stream.py and __future__/evaluator.py shims added The underlying API endpoints (/log_streams, /scorers) are unchanged; the server-side rename is tracked separately. Closes HYBIM-730 Co-authored-by: Cursor <cursoragent@cursor.com>
- Add return type -> object to all __getattr__ module functions - Add AgentStream to TYPE_CHECKING import in project.py so string annotations resolve correctly under mypy - Remove incorrect # type: ignore[override] from Evaluator.metrics property and add explicit return type BuiltInEvaluators Co-authored-by: Cursor <cursoragent@cursor.com>
- evaluator.py: remove deprecated `metrics` property; mypy forbids overriding a writable class attribute (Metric.metrics = BuiltInMetrics()) with a read-only property in a subclass. Evaluator.evaluators remains the canonical accessor. - project.py: add cast() around AgentStream.create() and AgentStream.list() return values. Both methods inherit LogStream's signature (-> LogStream / -> list[LogStream]); cast tells mypy the runtime types are AgentStream. Co-authored-by: Cursor <cursoragent@cursor.com>
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: request_changes — Public convenience functions for agent streams return broken objects; deprecation shims silently never fire.
General Comments
- 🟠 major (testing): The test plan lists a manual import/deprecation smoke test but no automated tests were added, and the checked items don't cover the module-level convenience functions (
get_agent_stream/list_agent_streams/create_agent_stream) or the module-level deprecation aliases — which is exactly where the bugs are. A rename PR advertising "full backward compatibility" should include tests that (1) round-trip an AgentStream through each convenience function and assert.name/.id/is_synced(), (2) assert each deprecated module symbol emitsDeprecationWarning, and (3) assertEvaluator.get()/.list()return the expected type. All three would have caught the issues flagged here.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
src/splunk_ao/evaluators.py:15-20: Import ordering:from galileo_core.schemas.logging.step import StepType(line 19) is placed after thesplunk_ao.*imports, which violates the isort/ruff grouping used elsewhere in the repo. Runruff/isort to reorder (third-party before first-party).src/splunk_ao/agent_streams.py:164-170:enable_evaluatorsdoesfrom splunk_ao.log_streams import LogStreamsinside the function body even thoughLogStreamsis already imported at module top (line 17). Drop the redundant local import.src/splunk_ao/project.py:1139-1157: The deprecatedcreate_log_stream/list_log_streamsdocstrings were rewritten to say "Create/List a new agent stream", which reads oddly for a method named*_log_stream. Consider keeping the summary line referring to log streams and letting the.. deprecated::note point to the agent-stream replacement, so the deprecation intent stays clear.
| result = get_log_stream(name=name, project_id=project_id, project_name=project_name) | ||
| if result is None: | ||
| return None | ||
| stream = AgentStream.__new__(AgentStream) | ||
| stream.__dict__.update(result.__dict__) | ||
| return stream |
There was a problem hiding this comment.
🔴 critical (bug): This produces a broken AgentStream. get_log_stream() returns the service-layer splunk_ao.log_streams.LogStream (a subclass of LogStreamResponse, decorated with @attrs.define → slots). Its fields (id, name, project_id, ...) live in slots, so result.__dict__ does not contain them — stream.__dict__.update(result.__dict__) copies essentially nothing. Combined with AgentStream.__new__ bypassing __init__ (so _sync_state, project_name, etc. are never set), the returned object raises AttributeError on stream.name, stream.id, str(stream), is_synced(). The two LogStream classes also have incompatible schemas. AgentStream already inherits a working get() from the object-centric LogStream that returns AgentStream instances via cls._from_api_response; delegate to it instead.
| result = get_log_stream(name=name, project_id=project_id, project_name=project_name) | |
| if result is None: | |
| return None | |
| stream = AgentStream.__new__(AgentStream) | |
| stream.__dict__.update(result.__dict__) | |
| return stream | |
| return AgentStream.get(name=name, project_id=project_id, project_name=project_name) |
🤖 Generated by the Astra agent
| log_streams = list_log_streams( | ||
| project_id=project_id, | ||
| project_name=project_name, | ||
| limit=limit, | ||
| starting_token=starting_token, | ||
| ) | ||
| result: builtins.list[AgentStream] = [] | ||
| for ls in log_streams: | ||
| s = AgentStream.__new__(AgentStream) | ||
| s.__dict__.update(ls.__dict__) | ||
| result.append(s) | ||
| return result |
There was a problem hiding this comment.
🔴 critical (bug): Same defect as get_agent_stream: list_log_streams() returns slotted service-layer LogStream objects, so s.__dict__.update(ls.__dict__) copies nothing and AgentStream.__new__ skips initialization. Every returned object is a broken AgentStream that raises AttributeError on attribute access. Delegate to the inherited classmethod, which already returns AgentStream instances.
| log_streams = list_log_streams( | |
| project_id=project_id, | |
| project_name=project_name, | |
| limit=limit, | |
| starting_token=starting_token, | |
| ) | |
| result: builtins.list[AgentStream] = [] | |
| for ls in log_streams: | |
| s = AgentStream.__new__(AgentStream) | |
| s.__dict__.update(ls.__dict__) | |
| result.append(s) | |
| return result | |
| return AgentStream.list( | |
| project_id=project_id, | |
| project_name=project_name, | |
| limit=limit, | |
| starting_token=starting_token, | |
| ) |
🤖 Generated by the Astra agent
| ls = create_log_stream(name=name, project_id=project_id, project_name=project_name) | ||
| stream = AgentStream.__new__(AgentStream) | ||
| stream.__dict__.update(ls.__dict__) | ||
| return stream |
There was a problem hiding this comment.
🔴 critical (bug): Same defect: create_log_stream() returns a slotted service-layer LogStream, so stream.__dict__.update(ls.__dict__) copies no fields and __init__/_sync_state are never set. Use the object-centric create path instead, which returns a fully-initialized AgentStream.
| ls = create_log_stream(name=name, project_id=project_id, project_name=project_name) | |
| stream = AgentStream.__new__(AgentStream) | |
| stream.__dict__.update(ls.__dict__) | |
| return stream | |
| return AgentStream(name=name, project_id=project_id, project_name=project_name).create() |
🤖 Generated by the Astra agent
| from splunk_ao.metric import ( | ||
| BuiltInMetrics, | ||
| CodeMetric, | ||
| LlmMetric, | ||
| LocalMetric, | ||
| Metric, | ||
| SplunkAOMetric, |
There was a problem hiding this comment.
🟠 major (bug): These top-level imports make Metric, LlmMetric, CodeMetric, LocalMetric, SplunkAOMetric, and BuiltInMetrics real module globals, so the deprecation __getattr__ at line 164 is never invoked for them (module __getattr__ only fires when normal attribute lookup fails). Accessing splunk_ao.evaluator.Metric returns the class with no DeprecationWarning, contradicting the module docstring and the PR's stated backward-compat contract. To make the shim work, import these under private names (e.g. from splunk_ao.metric import Metric as _Metric) so the deprecated public names resolve only via __getattr__, and reference the private names in the class bases below.
🤖 Generated by the Astra agent
| import datetime | ||
| import warnings | ||
|
|
||
| from splunk_ao.metrics import Metrics, create_custom_llm_metric, delete_metric, get_metrics |
There was a problem hiding this comment.
🟠 major (bug): create_custom_llm_metric, delete_metric, and get_metrics are imported as module globals here, so the deprecation __getattr__ at line 180 never fires for them — accessing splunk_ao.evaluators.get_metrics returns the function with no warning. Import them under private aliases (e.g. create_custom_llm_metric as _create_custom_llm_metric) so the deprecated names are only reachable through __getattr__.
🤖 Generated by the Astra agent
| import warnings | ||
|
|
||
| from splunk_ao.agent_stream import AgentStream | ||
| from splunk_ao.log_streams import LogStreams, create_log_stream, get_log_stream, list_log_streams |
There was a problem hiding this comment.
🟠 major (bug): create_log_stream, get_log_stream, and list_log_streams are imported as module globals, so the deprecation __getattr__ at line 193 only ever fires for enable_metrics (the one name not imported here). The other three deprecated aliases never emit a warning. Import them under private names so the deprecated symbols resolve only via __getattr__.
🤖 Generated by the Astra agent
There was a problem hiding this comment.
src/splunk_ao/evaluator.py:843-855 (line not in diff)
🟠 major (bug): Evaluator.get()/Evaluator.list() are inherited from Metric and route through Metric._create_metric_from_type, which hardcodes LlmMetric/CodeMetric/SplunkAOMetric — never the *Evaluator subclasses. So Evaluator.get(name="correctness") returns a SplunkAOMetric, and this docstring's assert isinstance(ev, SplunkAOEvaluator) is false. The rename doesn't surface new types on read paths. Either override _create_metric_from_type in Evaluator to return the Evaluator subclasses, or correct the docstrings to state that get()/list() return Metric-family instances.
🤖 Generated by the Astra agent
|
@adityamehra as discussed in the ticket, let's not add a deprecated shim layer, the plan is to do a pure renaming. The migration guide / tool will be used for customers that need to migrate their code. Backward compatibility is provided via the |
Summary
Implements the SDK-side domain entity renames from HYBIM-730:
AgentStreamreplacesLogStreamas the canonical agent-stream classEvaluator(andLlmEvaluator,CodeEvaluator,LocalEvaluator,SplunkAOEvaluator) replacesMetricand its subclassesDeprecationWarningand remain functionalThe underlying API endpoints (
/log_streams,/scorers) are unchanged; the server-side rename is tracked separately.New API at a glance
Backward compatibility
Old names remain functional via PEP 562
__getattr__in__init__.py:Files changed
src/splunk_ao/agent_stream.pyAgentStreamsubclass ofLogStreamsrc/splunk_ao/agent_streams.pyAgentStreamsservice +get_agent_stream,list_agent_streams,create_agent_stream,enable_evaluatorssrc/splunk_ao/evaluator.pyEvaluator,LlmEvaluator,CodeEvaluator,LocalEvaluator,SplunkAOEvaluatorsrc/splunk_ao/evaluators.pyEvaluatorsservice +create_custom_llm_evaluator,get_evaluators,delete_evaluatorsrc/splunk_ao/__future__/agent_stream.pysrc/splunk_ao/__future__/evaluator.pysrc/splunk_ao/__init__.py__getattr__for old namessrc/splunk_ao/project.pycreate_agent_stream,list_agent_streams,agent_streams; deprecate old counterpartsdocs/HYBIM-730-domain-entity-rename.mdTest plan
splunk_ao.LogStreamandsplunk_ao.MetricemitDeprecationWarningAgentStream→LogStream→StateManagementMixin;Evaluator→Metric→StateManagementMixinAgentStreamagainst a live environmentProject.create_agent_streamround-trips correctlyRelated
Made with Cursor