Skip to content

Add memory subgraph example and reporting integration#596

Merged
jfrench9 merged 2 commits intomainfrom
chore/subgraph-memory-example
Apr 5, 2026
Merged

Add memory subgraph example and reporting integration#596
jfrench9 merged 2 commits intomainfrom
chore/subgraph-memory-example

Conversation

@jfrench9
Copy link
Copy Markdown
Member

@jfrench9 jfrench9 commented Apr 5, 2026

Summary

Introduces a new memory subgraph example demonstrating how to create and manage memory-based subgraphs, along with integration points for reporting this capability to Dagster. This is a chore/example addition that expands the custom graph demo suite.

Key Accomplishments

  • New memory subgraph example: Added a comprehensive example (07_memory_subgraph.py) showcasing memory subgraph creation patterns, significantly expanding the custom graph demo library with ~540 lines of example code
  • Demo entrypoint update: Updated the custom graph demo main module to include and expose the new memory subgraph example
  • Subgraph router registration: Extended the subgraphs router to register the memory subgraph capability, enabling it to be discovered and utilized within the broader graph infrastructure
  • Audit logging enhancement: Added audit logging support for the new memory subgraph operations to maintain security observability

Breaking Changes

None. This is an additive change that introduces a new example and extends existing modules without modifying current behavior.

Testing Notes

  • Verify the new memory subgraph example runs correctly through the custom graph demo entrypoint
  • Confirm the subgraph router properly registers and routes to the new memory subgraph
  • Validate that audit log entries are correctly emitted for memory subgraph operations
  • Ensure existing graph demos and subgraph functionality remain unaffected by the additions

Infrastructure Considerations

  • The new subgraph type should be reflected in any orchestration or pipeline management systems (e.g., Dagster) that discover and report on available graph types
  • Audit logging infrastructure should be verified to handle the new event types without requiring schema or storage changes
  • No new dependencies or deployment configuration changes are expected

🤖 Generated with Claude Code

Branch Info:

  • Source: chore/subgraph-memory-example
  • Target: main
  • Type: chore

Co-Authored-By: Claude noreply@anthropic.com

- Implemented step 7 in the custom graph demo to create a memory subgraph.
- Updated the subgraph creation process to report asset materialization to Dagster, including relevant metadata.
- Added new security event types for subgraph creation and deletion in the audit logger.
@jfrench9
Copy link
Copy Markdown
Member Author

jfrench9 commented Apr 5, 2026

@claude please review this PR

@claude
Copy link
Copy Markdown

claude Bot commented Apr 5, 2026

Claude finished @jfrench9's task in 2m 10s —— View job


PR Review: Add memory subgraph example and reporting integration

  • Gather context and understand request
  • Read changed files
  • Analyze code quality and provide feedback

Overall this is a clean, additive PR. The example is well-structured and the Dagster reporting integration follows the established pattern in reporting.py. A few issues worth addressing below.


Critical: origin/main was already broken before this PR

robosystems/routers/graphs/subgraphs/main.py on main references SecurityEventType.SUBGRAPH_CREATED (lines 355 and 394), but that enum value didn't exist in audit_logger.py until this PR adds it. This means main had a broken AttributeError at import time. This PR implicitly fixes it, but it should be called out explicitly — and ideally the previous PR that introduced the router code should have included this enum addition.


Issues

1. 07_memory_subgraph.py--credentials-file arg ignored for credential loading

load_credentials() hardcodes CREDENTIALS_FILE rather than accepting a path argument:

# examples/custom_graph_demo/07_memory_subgraph.py:61-66
def load_credentials() -> dict:
    if not CREDENTIALS_FILE.exists():  # ← always uses the constant
        ...

# main():
credentials = load_credentials()      # ← ignores args.credentials_file
graph_id = get_graph_id(credentials_path, DEMO_NAME)  # ← credentials_path used here only

Passing --credentials-file /custom/path.json will load credentials from .local/config.json instead. Other scripts in this suite (e.g., 01_setup_credentials.py) likely pass the path through to the loader. Fix this →

2. main.py — step 7 runs unconditionally, will fail on re-runs

Step 6 (document upload) is guarded by a directory existence check. Step 7 runs unconditionally:

# examples/custom_graph_demo/main.py:172-179
# Step 7: Create memory subgraph
step7_args = [...]
run_script("07_memory_subgraph.py", step7_args)  # ← no guard

07_memory_subgraph.py calls sys.exit(1) if subgraph creation fails (e.g. duplicate name on re-run), which propagates through run_script and aborts the whole demo. Consider either adding --skip-memory-subgraph flag or handling the "already exists" case gracefully in the example script. Fix this →

3. subgraphs/main.py — fork path missing Dagster reporting

The new Dagster report_asset_materialization call is only in the non-fork code path. Subgraphs created via fork_parent=True return early (line ~377) without reporting:

# robosystems/routers/graphs/subgraphs/main.py
if request.fork_parent:
    ...
    return response   # ← no Dagster reporting here

Forked subgraphs won't appear in Dagster observable assets. Intentional or oversight?


Minor Notes

4. SUBGRAPH_DELETED added but unused

SUBGRAPH_DELETED = "subgraph_deleted" is added to the enum but nothing in this PR emits it. Fine as forward-looking prep, but if there's no deletion endpoint yet it could be left until that's implemented to keep the enum tidy.

5. asset_key="user_subgraph_creation" — verify asset exists in Dagster definitions

The hardcoded string "user_subgraph_creation" should match an observable source asset key defined somewhere in robosystems/dagster/. A missing definition won't raise an error (the call is wrapped in try/except) but the materialization will be silently orphaned. Worth a quick grep to confirm it's defined.


What's Good

  • The example script is thorough and well-organized — covers creation, seeding (Concepts, Observations, Sessions), relationships, and verification queries
  • The Dagster reporting block correctly wraps the call in try/except with a logger.warning, consistent with the established pattern in reporting.py
  • SUBGRAPH_CREATED is already used in the router on main — adding it to the enum here actually fixes a latent AttributeError
  • Emojis in the example script are appropriate per CLAUDE.md (examples/ is exempt from the no-emoji rule)
  • No new dependencies, migrations, or breaking changes

|

Comment thread examples/custom_graph_demo/07_memory_subgraph.py Fixed
- Updated the `load_credentials` function to accept a path parameter for improved flexibility.
- Added handling for duplicate subgraph creation in the `create_memory_subgraph` function.
- Enhanced error reporting in the `execute_query` function to include exception details.
- Adjusted import statements for better organization in the custom graph demo script.
- Added a comment in the subgraph creation process to clarify Dagster reporting behavior.
@jfrench9 jfrench9 merged commit ebca4e4 into main Apr 5, 2026
7 checks passed
@jfrench9 jfrench9 deleted the chore/subgraph-memory-example branch April 5, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant