Docs/update agents md

[pradystar]

Summary

• Replace AGENTS.md with current repository wide development guidance
• Add ARCHITECTURE.md covering SDK boundaries, telemetry paths, configuration and testing
• Add CLAUDE.md importing the shared agent instructions
• Document approval requirements for commands, workflows, Git operations and subagents

[shuningc]

🤖 This review was generated by the Astra agent (claude-opus-5). It may contain mistakes.

Verdict: request_changes — The A2A/ADK lint commands listed as non-mutating actually rewrite files (ruff fix = true in both subpackage configs), which contradicts the permission model this PR is establishing; several other factual claims about paths, lockfiles, and export flow are inaccurate.

General Comments

• 🟡 minor (documentation): These docs are instruction files an agent will treat as authoritative, so factual drift is the main risk rather than prose quality. Most claims check out (Poetry 2.4.1, Python 3.11–3.14, root CI on Linux/macOS/Windows, pytest 9, OTel 1.38, galileo-core ^4.4.0, resolve_deployment() behavior, flush()/terminate() semantics, NormalizingSpanExporter in exporter/config.py), and replacing the stale SPLUNK_AO_SF_TOKEN guidance with the real SPLUNK_AO_O11Y_TOKEN/SPLUNK_AO_O11Y_API_TOKEN pair is a genuine correctness fix. The remaining issues are concentrated in a few specific claims flagged inline.

One process suggestion: since the value of these files depends entirely on staying true, consider noting in CONTRIBUTING.md (or the change workflow section) that AGENTS.md/ARCHITECTURE.md need updating when telemetry paths, env vars, or package layout change. Otherwise this content will drift silently and agents will act on stale invariants — exactly the failure mode the old SPLUNK_AO_SF_TOKEN line represented.

Follow-ups

Suggested follow-up work that could be tracked as Jira tickets:

• docs/domain-entity-rename.md:1-1: ARCHITECTURE.md states "docs/ contains repository documentation", but docs/ holds a single file (domain-entity-rename.md) while generated API references land in the gitignored .generated_docs/ (see .github/workflows/publish-docs.yaml:31-36). Consider either consolidating repository docs under docs/ so the statement is substantive, or trimming the claim. Not blocking — the sentence does distinguish generated output from checked-in docs.
• AGENTS.md:9-10: The permission model gates every project command behind explicit approval, including read-only ones like poetry run pytest and poetry run mypy. This is a deliberate policy choice and outside this review's scope, but it will produce a lot of approval round-trips for ordinary test-driven work. Worth revisiting whether a small allowlist of non-mutating verification commands (targeted pytest, mypy, ruff check --no-fix) could be pre-approved while keeping installs, lock updates, codegen, and Git/workflow operations gated.

[shuningc]

🟠 major (bug): uv run ruff check src tests is a mutating command here, but it's listed in the "Reference Only" block alongside commands explicitly labeled non-mutating.

splunk-ao-a2a/pyproject.toml:86 sets fix = true under [tool.ruff], so a bare ruff check auto-applies fixes and rewrites files. The ADK block on line 67 has the identical problem (splunk-ao-adk/pyproject.toml:57).

This matters because the whole point of lines 42-43 and 71-78 is to draw a hard mutating/non-mutating boundary. The root commands correctly use --no-fix and format --check, which shows the author knew about fix = true — the subpackage blocks just didn't get the same treatment. As written, an agent following this guidance will silently modify files in splunk-ao-a2a/ and splunk-ao-adk/ while believing it ran a read-only check.

Apply --no-fix to both subpackage blocks (line 56 and line 67).

Suggested change

	uv run ruff check src tests
	uv run ruff check --no-fix src tests

🤖 Generated by the Astra agent

[shuningc]

🟡 minor (documentation): All paths -> NormalizingSpanExporter -> deployment-aware OTLP exporter -> backend is not true for Paths 2 and 3 — it holds unconditionally only for Path 1.

start_splunk_ao_span() (src/splunk_ao/otel.py:318-323) resolves a provider from _TRACE_PROVIDER_CONTEXT_VAR, falling back to trace.get_tracer_provider(). It never constructs a Splunk exporter itself. Spans reach NormalizingSpanExporter only if the caller previously registered add_splunk_ao_span_processor() (which is what populates that context var at otel.py:301). examples/logging-samples/distributed-tracing-otel-python-java/python-service/app.py demonstrates the opposite case: it uses start_splunk_ao_span() with a plain OTLPSpanExporter pointed at a collector, so those spans never touch NormalizingSpanExporter at all.

As written, an agent could reasonably conclude Path 2 is self-configuring for export and skip wiring or testing processor registration. Worth making the dependency explicit.

Suggested change

	Path 2: start_splunk_ao_span()
	-> SDK-created OTel spans
	Path 3: external OTel/OpenInference + A2A
	-> caller TracerProvider
	-> add_splunk_ao_span_processor(provider)
	All paths -> NormalizingSpanExporter -> deployment-aware OTLP exporter -> backend
	Path 1: decorator / logger / handlers / OpenAI / ADK
	-> internal LoggedTrace + completed steps
	-> SpanConverter
	-> OTel ReadableSpan copies
	-> SDK-owned SpanSink
	Path 2: start_splunk_ao_span()
	-> SDK-created OTel spans on the provider in context
	(requires a registered Splunk AO processor to reach the backend)
	Path 3: external OTel/OpenInference + A2A
	-> caller TracerProvider
	-> add_splunk_ao_span_processor(provider)
	Path 1 always, and Paths 2-3 once a Splunk AO processor is registered
	-> NormalizingSpanExporter -> deployment-aware OTLP exporter -> backend

🤖 Generated by the Astra agent

[shuningc]

🟡 minor (documentation): "independent versions, lockfiles" is inaccurate for the A2A and ADK packages: uv.lock is gitignored (.gitignore:103) and neither splunk-ao-a2a/uv.lock nor splunk-ao-adk/uv.lock exists in the repo. Only the root poetry.lock is committed.

This interacts with two other statements — "Do not hand-edit ... generated lock content" (line 190) and "Do not edit versions, release workflows, or lockfiles as incidental cleanup" (line 186). An agent may go looking for subpackage lockfiles that aren't there, or assume uv sync --dev output should be committed.

Suggested change

	The three buildable packages have independent versions, lockfiles, CI, and release workflows. Validate every package a
	The three buildable packages have independent versions, CI, and release workflows. Only the root `poetry.lock` is
	committed; `uv.lock` is gitignored, so A2A and ADK resolve dependencies at sync time. Validate every package a
	change touches. CI supports Python 3.11–3.14; root CI also spans Linux, macOS, and Windows.

🤖 Generated by the Astra agent

[shuningc]

🟡 minor (documentation): This row overstates SPLUNK_AO_O11Y_TOKEN as required, and contradicts ARCHITECTURE.md lines 416-425 in the same PR.

O11yConfig.__post_init__ (src/splunk_ao/deployment.py:61-64) requires one of SPLUNK_AO_O11Y_TOKEN or SPLUNK_AO_O11Y_API_TOKEN. README.md:98-100 confirms a CRUD-only setup is SPLUNK_AO_REALM + SPLUNK_AO_O11Y_API_TOKEN with no ingest token. The ARCHITECTURE.md table splits CRUD vs telemetry auth correctly; this condensed version loses that distinction and implies the ingest token is mandatory even for CRUD-only use.

Since the header is "Required authentication", the split matters — an agent could reject a valid CRUD-only config as misconfigured.

Suggested change

	| O11y Cloud | `SPLUNK_AO_REALM` plus `SPLUNK_AO_O11Y_TOKEN`; that token may serve CRUD when permitted, or use a dedicated `SPLUNK_AO_O11Y_API_TOKEN` |
	| O11y Cloud | `SPLUNK_AO_REALM` plus at least one of `SPLUNK_AO_O11Y_TOKEN` (required for telemetry export) or `SPLUNK_AO_O11Y_API_TOKEN` (CRUD only); see `ARCHITECTURE.md` for the CRUD/telemetry split |

🤖 Generated by the Astra agent

[shuningc]

🟡 minor (documentation): These two paths are wrong. Both files live in a subpackage: src/splunk_ao/converter/attribute_mapping.py and src/splunk_ao/converter/span_converter.py.

Every other row in this table either names a file directly under src/splunk_ao/ (otel.py, config.py) or names the package with a trailing slash (logger/, exporter/, resources/), so a reader will correctly infer these are at src/splunk_ao/attribute_mapping.py — which does not exist. The same omission appears in the Change-Impact Map on line 207.

Suggested change

	| Conversion | `attribute_mapping.py`, `span_converter.py` | Internal completed steps to OTel `ReadableSpan` data |
	| Conversion | `converter/attribute_mapping.py`, `converter/span_converter.py` | Internal completed steps to OTel `ReadableSpan` data |

🤖 Generated by the Astra agent

[shuningc]

🔵 nit (documentation): The paraphrased state names don't match the enum, which defeats grep-ability. Actual SyncState members (src/splunk_ao/shared/base.py:28-32) are LOCAL_ONLY, SYNCED, DIRTY, FAILED_SYNC, DELETED — note DIRTY, not "modified". Using the real identifiers lets an agent jump straight to the code this section is meant to route it toward.

Suggested change

	Object resources use lifecycle states through `StateManagementMixin` (local, synced, modified, failed, deleted). Keep
	Object resources use lifecycle states through `StateManagementMixin` (`SyncState.LOCAL_ONLY`, `SYNCED`, `DIRTY`,
	`FAILED_SYNC`, `DELETED`). Keep

🤖 Generated by the Astra agent

[shuningc]

🔵 nit (documentation): Same path issue as line 30 — attribute_mapping.py is at src/splunk_ao/converter/attribute_mapping.py.

Suggested change

	| Span attributes/content | schemas, `attribute_mapping.py`, converter, span transform | Path 1 plus any affected native/external path |
	| Span attributes/content | schemas, `converter/attribute_mapping.py`, converter, span transform | Path 1 plus any affected native/external path |

🤖 Generated by the Astra agent

[shuningc]

🔵 nit (documentation): The @AGENTS.md import is the right call — it avoids duplicating shared instructions. But the empty ## Claude Code section holding only an HTML comment adds nothing today and is the kind of placeholder that tends to sit empty indefinitely.

Suggest either dropping the heading (leaving just the import plus the comment about where shared instructions live), or filling it with something real if there is genuinely Claude-specific guidance to record.

🤖 Generated by the Astra agent

[shuningc]

🟡 minor (documentation): Some concrete, still-accurate guidance from the old file was dropped and replaced with generic statements, which reduces the practical value for the exact tasks it was helping with:

1. CrewAI test setup. The old file said: patch _crewai_imports_resolved / CREWAI_AVAILABLE, mock AgentStreams/Projects/Traces, and pass a mock SplunkAOLogger (see tests/test_crewai_handler.py). I verified all of these are still exactly what the tests do (tests/test_crewai_handler.py:83-85,93,100). The new text reduces this to "Preserve lazy imports and test both installed/unavailable behavior" (line 173), which does not tell an agent how. This is non-obvious setup that is expensive to rediscover.

2. Two real gotchas removed from "Known Issues" that I confirmed still hold:

   • Experiment conflates experiment and playground concepts (src/splunk_ao/experiment.py:902,1506,1520-1531).
   • Handlers stringify metadata values, and Trace vs Dataset APIs differ (handlers/langchain/handler.py:168, handlers/agent_control/bridge.py:76-87).

   The 1-based dataset version and split config state gotchas survived (lines 174, 127-128), so the omission of these two looks unintentional rather than a deliberate trim.

Suggest restoring the CrewAI test recipe under "Testing Rules" and the two gotchas as a short "Known Sharp Edges" list. If they were dropped deliberately as internal context, that's fine — but they're all observable from public source, so they don't conflict with the "documentation must stand alone for public contributors" boundary on line 194.

🤖 Generated by the Astra agent