fix(docs): replace stale log_stream references in published docstrings - #213
fix(docs): replace stale log_stream references in published docstrings#213etserend wants to merge 1 commit into
Conversation
…amples (HYBIM-941) - agent_stream.py: all example blocks now use agent_stream.* calls - agent_streams.py: enable_evaluators example blocks updated - singleton.py: rename log_stream/log_stream_id param docs to agent_stream/agent_stream_id across get(), reset(), flush(); fix body prose "log_stream" → "agent_stream" in flush() description - decorator.py: rename log_stream/log_stream_id param docs to agent_stream/agent_stream_id in get(), flush(), and reset() methods Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-5). It may contain mistakes.
Verdict: request_changes — The linked Jira ticket (HYBIM-941) describes a completely unrelated, currently-blocked task, and the docstring sweep misses ~20 stale references — including three that reference the deleted LogStream class — in files that feed the same docs publish, so the PR's stated goal of a clean Codex review will not be met.
General Comments
- 🟠 major (question): The linked Jira ticket does not describe this change.
HYBIM-941 is titled "Rebrand console.galileo.yourcompany.com placeholder in example env configs". Its acceptance criteria name 4 specific occurrences:
examples/logging-samples/log-mcp-calls/README.md:33examples/logging-samples/log-mcp-calls/.env.example:12examples/logging-samples/openai-responses/README.md:39examples/agent/google-adk/my_agent/.env.example:13
This PR touches none of those files, and none of the files it does touch are mentioned in the ticket. The ticket is also explicitly marked Blocked: "This story is blocked until we know what the URL structure for stand-alone Splunk Agent Observability deployments will look like... this is a product-naming decision, not a mechanical find/replace."
The PR description gives the real provenance — "Codex review findings from agent-observability-docs PR #41" — which is legitimate work, but it is not HYBIM-941.
This matters beyond bookkeeping: HYBIM-941 will now be closed by a PR that did not satisfy any of its acceptance criteria, so the actual placeholder rebrand silently falls off the board while still blocked. Please either open a new ticket for the docstring rename and retarget this PR, or find the correct existing ticket. I'd avoid repurposing a blocked ticket for unrelated work.
- 🟠 major (documentation): The sweep is incomplete — the stated goal (a clean Codex review on the next docs publish) will not be achieved.
scripts/create_docs.py (line 819-823) walks all of ./src/splunk_ao, ignoring only __pycache__, constants, resources, schema, __future__. Everything else — including shared/ — is published to the same docs site by .github/workflows/publish-docs.yaml. So these remaining stale references land in the same generated .mdx output and will re-trigger the same findings:
| File | Stale refs |
|---|---|
src/splunk_ao/shared/column.py |
lines 62, 65, 67-68, 70, 171, 174, 456, 476, 525, 528, 531, 535, 539 |
src/splunk_ao/shared/query_result.py |
lines 65-66, 81, 193, 199, 246, 273 |
src/splunk_ao/evaluator.py |
line 1245 |
src/splunk_ao/agent_stream.py (this PR) |
lines 333, 336, 339, 342 |
src/splunk_ao/agent_streams.py (this PR) |
lines 65-66 |
The shared/ ones are more severe than anything fixed here (see the line comment on column.py:62 — they reference the deleted LogStream class). Note that two of these files were already edited by this PR, so those misses aren't a scope question — just an incomplete pass.
A grep -rn 'log_stream\.\|log_streams = \|LogStream\.' src/splunk_ao --include='*.py' (excluding resources/, schema/, constants/) before merge would catch all of these.
- 🟡 minor (documentation): Prose still says "log stream" throughout, so the docs read inconsistently even where identifiers were fixed.
This PR updated prose in singleton.py and decorator.py, but ~100+ user-facing "log stream" / "Log stream" prose occurrences remain in docstrings that get published — e.g. agent_stream.py (class summary at lines 49-64, plus 113, 159, 253, 309, 362, 411, 447, 506, 606, 659, 708, 759, 823, 869, 902, 936), agent_streams.py (lines 25-45, 110, 129-143, 255-274, 369-385, 430-444, 487-516), utils/env_helpers.py:60-122, openai/__init__.py:308.
The result is a docs page titled around AgentStream whose every method description says "this log stream". Since the goal is a clean Codex review, partial prose coverage is likely to surface the same class of finding again.
I'd suggest picking one of two explicit scopes rather than the current middle ground:
- Identifiers only — revert the prose changes in
singleton.py/decorator.py, and file a follow-up for all prose. Smallest, most reviewable diff. - Identifiers + all published prose — finish the prose sweep in this PR.
Option 2 matches the PR's intent better. Either way, keep internal names (log_stream_id API fields, LOG_STREAM_TARGET_TYPE, route paths) untouched — per docs/domain-entity-rename.md, the server still uses /log_streams.
Follow-ups
Suggested follow-up work that could be tracked as Jira tickets:
src/splunk_ao/agent_control.py:20-20:LOG_STREAM_TARGET_TYPE = "log_stream"and the surrounding docstrings (lines 35, 62-63, 79) expose the wire-levellog_streamtarget-type string in user-facing error messages and docs — e.g."agent_stream_id can only be used with target_type='log_stream'."This is correct today because the server still useslog_stream(perdocs/domain-entity-rename.md), so it is deliberately out of scope here. Worth tracking alongside the server-side rename so the publictarget_typevalue and the SDK'sagent_streamvocabulary converge.scripts/create_docs.py:819-823: Consider adding a CI guard that fails when published docstrings contain stale identifiers, so this class of drift is caught mechanically instead of by downstream Codex review on the docs repo. A cheap version: grep the generated.mdxoutput under.generated_docs/referenceforlog_stream.,log_streams =, andLogStream.and fail thepublish-docsjob (or a CI job) on any hit. This would have caught every finding in this review, plus theLogStreamreferences to the deleted class.src/splunk_ao/shared/query_result.py:105-105:query_fn: The function to call for pagination (e.g., log_stream.query)— stale reference in a private__init__docstring. Not published (create_docs.pyskips underscore-prefixed methods), so non-blocking; worth cleaning up with any future pass over this file.
There was a problem hiding this comment.
src/splunk_ao/decorator.py:215-216 (line not in diff)
🟡 minor (documentation): This param entry is malformed — the description is duplicated, and the first line uses inline name: description style while every sibling param in this block (project, experiment_id, mode, session_id) uses the NumPy convention of name on one line and indented description on the next.
create_docs.py renders params as - \{name}`: {desc}(line 606), sodocstring_parserwill read the name asagent_streamwith description"The log stream name to use for this context\nThe log stream name to use for this context"`, emitting a bullet with the sentence twice. Worth fixing while in this file, since it is the same published-docs defect class the PR targets.
| agent_stream | |
| The agent stream name to use for this context |
🤖 Generated by the Astra agent
There was a problem hiding this comment.
src/splunk_ao/agent_stream.py:332-342 (line not in diff)
🟡 minor (documentation): Missed in this file's sweep: the result variable is still log_streams. Unlike the other cases this is a local assignment so it isn't a broken example, but it is the same stale-naming finding Codex flagged, in a file this PR already edited.
Also note the prose on lines 309, 319, 324, 341 still says "log streams" — see the PR-level comment on prose scope.
| # List by project name | |
| agent_streams = AgentStream.list(project_name="My AI Project") | |
| # List by project ID | |
| agent_streams = AgentStream.list(project_id="project-123") | |
| # List using SPLUNK_AO_PROJECT environment variable | |
| agent_streams = AgentStream.list() | |
| # Cap the number of returned agent streams | |
| agent_streams = AgentStream.list(project_name="My AI Project", limit=3) |
🤖 Generated by the Astra agent
There was a problem hiding this comment.
src/splunk_ao/agent_streams.py:63-67 (line not in diff)
🟡 minor (documentation): Missed in this file's sweep — log_streams variable and the "Log Stream: ..." log message. This is the module-level class docstring, which is published.
| # List all agent streams in a project | |
| from splunk_ao.agent_streams import list_agent_streams | |
| agent_streams = list_agent_streams(project_name="My AI Project") | |
| for stream in agent_streams: | |
| logger.info(f"Agent Stream: {stream.name} (ID: {stream.id})") |
🤖 Generated by the Astra agent
Summary
Fixes stale
log_stream.*variable references and param documentation in published docstrings — left over from theLogStream → AgentStreamrename. These are docstring-only changes; no runtime behaviour is affected.Source: Codex review findings from agent-observability-docs PR #41. The SDK's docstrings are auto-published via
.github/workflows/publish-docs.yaml/scripts/create_docs.py— the generated.mdxfiles in that repo must not be manually patched; the fix belongs here.Files changed
src/splunk_ao/agent_stream.pylog_stream.*→agent_stream.*src/splunk_ao/agent_streams.pyenable_evaluatorsexample blocks updatedsrc/splunk_ao/utils/singleton.pyget(),reset(),flush()src/splunk_ao/decorator.pyget(),flush(),reset()methodsNote: This PR covers the identifiers and param docs flagged in the initial Codex review. Remaining stale prose (
shared/column.py,shared/query_result.py,evaluator.py, and broader "log stream" prose throughout published docstrings) is out of scope here and will be addressed in a follow-up.Test plan
grep -r "log_stream\." src/splunk_ao/agent_stream.pyreturns onlyretrieved_log_stream.*/created_log_stream.*(internal API response variables, correct)agent-observability-docs🤖 Generated with Claude Code