Skip to content

trace: emit agent-core events from NOC graph#61

Merged
Svaag merged 1 commit into
mainfrom
trace/agent-core-emitter
Jun 29, 2026
Merged

trace: emit agent-core events from NOC graph#61
Svaag merged 1 commit into
mainfrom
trace/agent-core-emitter

Conversation

@Svaag

@Svaag Svaag commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a best-effort, flag-gated NOC agent-core TraceEvent emitter using agent_core.tracing.sink_from_env
  • emit graph summaries, proposal events, evidence summaries, and operator decision/resume results
  • hook emission after graph completion and operator decision recording without changing remediation policy gates
  • bump agent-core dependency to v0.4.0

Validation

  • uv run pytest tests/test_agent_core_trace.py tests/test_graph_runtime.py
  • uv run ruff check app/agent_core_trace.py app/graph_runtime.py tests/test_agent_core_trace.py tests/test_graph_runtime.py
  • uv run mypy app/agent_core_trace.py
  • uv run pytest
  • uv run ruff check app tests
  • uv run mypy app

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 90
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle non-list iterable state values

In emit_state_trace, the _listish calls assume dictionary keys exist unconditionally
(e.g., len(_listish(state.get("proposals")))); if any underlying value is not a list
but is iterable (e.g., a tuple), _listish will return [] and incorrectly report zero
count. Convert non-list iterables to lists to count accurately.

app/agent_core_trace.py [159-161]

 def _listish(value: Any) -> list[Any]:
-    return list(value) if isinstance(value, list) else []
+    if isinstance(value, list):
+        return value
+    try:
+        return list(value)
+    except TypeError:
+        return []
Suggestion importance[1-10]: 7

__

Why: The improved code correctly converts any iterable to a list, fixing underestimation of counts when state values are tuples or other iterable types. This is a robust improvement with moderate impact.

Medium
General
Improve JSON serialization safety

The double serialization via json.loads(json.dumps(...)) is inefficient and can
cause data loss for nested objects with custom str methods (strings replaced
instead of actual values). Consider a recursive sanitization approach that only
converts non-serializable types to strings, keeping the original structure intact.

app/agent_core_trace.py [170-171]

 def _jsonish(value: Any) -> Any:
-    return json.loads(json.dumps(value, default=str))
+    if isinstance(value, dict):
+        return {k: _jsonish(v) for k, v in value.items()}
+    if isinstance(value, (list, tuple)):
+        return [_jsonish(v) for v in value]
+    if isinstance(value, (str, int, float, bool, type(None))):
+        return value
+    try:
+        json.dumps(value)
+        return value
+    except (TypeError, ValueError):
+        return str(value)
Suggestion importance[1-10]: 7

__

Why: The recursive sanitization avoids double serialization and preserves the original structure, preventing data loss from custom __str__ methods and improving efficiency. This is a valid moderate enhancement.

Medium

@Svaag Svaag force-pushed the trace/agent-core-emitter branch from b030457 to c1ff300 Compare June 29, 2026 15:39
@Svaag

Svaag commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the PR-agent suggestions in c1ff300:

  • _listish now handles tuple state values (without accidentally expanding strings/dicts).
  • _jsonish now recursively preserves JSON-native structure and only stringifies non-serializable leaf values.
  • Added regression coverage for tuple proposal/evidence values and unknown object sanitization.

Validation after the fix: uv run pytest tests/test_agent_core_trace.py tests/test_graph_runtime.py, uv run ruff check app/agent_core_trace.py tests/test_agent_core_trace.py, uv run mypy app/agent_core_trace.py, full uv run pytest, full uv run ruff check app tests, full uv run mypy app.

@Svaag Svaag merged commit 72c21b0 into main Jun 29, 2026
4 of 5 checks passed
@Svaag Svaag deleted the trace/agent-core-emitter branch June 29, 2026 15:42

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b030457ef1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pyproject.toml
"discord.py>=2.5.0",
"pyjwt[crypto]>=2.9",
"asyncpg>=0.30",
"agent-core",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep trace-only agent-core out of required installs

Because the tracing code imports agent_core lazily and returns before importing it unless HYRULE_NOC_AGENT_CORE_TRACE is enabled, this required dependency now makes normal installs and the CI uv sync --frozen --group dev path fetch https://github.com/AS215932/agent-core even when tracing is off. In environments without GitHub egress/credentials, the whole app/test install fails before the best-effort flag gate can apply; make this an optional extra or otherwise ensure it is available from the normal dependency source.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant