feat(DATAGO-131493): User can run the experiment and view the result#1473
Merged
feat(DATAGO-131493): User can run the experiment and view the result#1473
Conversation
Two strict additions enable the enterprise eval pipeline to persist the
captured A2A event stream alongside its existing execution-data blob and
to run end-to-end in local-dev environments without S3.
1. Public visualization mapper utility
- New `gateway/http_sse/visualization_mapper.py` exporting the pure
function `infer_visualization_event_details(topic, payload, log_identifier="")`,
lifted from `WebUIBackendComponent._infer_visualization_event_details`.
- The component method now delegates to this utility — zero behavior
change for chat. The eval pipeline (in enterprise) calls the same
utility at upload time so the persisted event shape matches what
chat streams live; no separate frontend mapping path.
2. Filesystem object-storage backend
- New `services/platform/storage/filesystem_client.py` —
`FileSystemStorageClient` implementing `ObjectStorageClient` against
a local directory (one file per object, file:// URLs).
- `factory.py` accepts `"filesystem"` / `"fs"` / `"local"` aliases.
Default remains `"s3"`; existing callers are unaffected.
- Lets enterprise initialize its eval storage service with no S3 infra
when `EVAL_DATA_BUCKET_NAME` is unset, so activity diagrams and
execution-data views work in a developer's local SAM out of the box.
Tests:
- `tests/unit/gateway/http_sse/test_visualization_mapper.py` — 7 tests
pinning the contract of the standalone function (system events,
topic-fallback for request/response/status, payload-summary
truncation, serialization-failure handling).
- `tests/unit/shared/storage/test_filesystem_storage.py` — 11 tests
covering the real disk path: round-trip put/get, factory aliases,
delete-prefix subtree behavior, missing-key 404, list_objects, and
file:// URLs. Lives in its own module because the existing
`test_object_storage.py` imports the optional Azure SDK at module top.
✅ FOSSA Guard: Licensing (
|
✅ FOSSA Guard: Vulnerability (
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extracts the SSE visualization event “details” mapper into a reusable pure function and introduces a local filesystem-backed ObjectStorageClient to unblock dev/eval workflows without requiring S3/MinIO.
Changes:
- Added
infer_visualization_event_details(topic, payload, log_identifier="")as a standalone utility and updated the Web UI component to delegate to it. - Added
FileSystemStorageClientplus factory support forfilesystem/fs/localbackends. - Added focused unit tests for the mapper utility and filesystem storage backend.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/solace_agent_mesh/gateway/http_sse/visualization_mapper.py |
New pure visualization mapping utility extracted from the component. |
src/solace_agent_mesh/gateway/http_sse/component.py |
Delegates visualization mapping to the new utility to keep behavior consistent. |
src/solace_agent_mesh/services/platform/storage/filesystem_client.py |
New filesystem-backed object storage client implementation. |
src/solace_agent_mesh/services/platform/storage/factory.py |
Adds filesystem backend aliases and wiring via env var root. |
tests/unit/gateway/http_sse/test_visualization_mapper.py |
Unit tests pinning the mapper utility’s contract. |
tests/unit/shared/storage/test_filesystem_storage.py |
Unit tests for filesystem storage behavior and factory aliases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
josephabonasara
changed the title
josephabonasara
requested review from
bwiebe-solace
and removed request for
bwiebe-solace
Address review feedback on the dev/test filesystem backend: - Resolve _root to an absolute path in __init__ so as_uri() (used by generate_presigned_url / get_public_url) works when callers pass a relative root_path. - Reject empty keys, absolute keys, and any key that resolves outside the bucket root in _path_for. Defense-in-depth — current callers pass server-constructed keys, but the class is part of the public storage interface. - Use Path.as_posix() in list_objects and the delete_prefix string- match fallback so returned/compared keys use '/' on every platform, matching the convention of the cloud backends. Tests: - TestRelativeRootPath verifies a relative root produces a usable file:// URL. - TestPathTraversalRejected covers ../, /etc/passwd, mid-key escape, empty key, and a "doesn't leak through get_object" case. - TestListAndUrls extended with an explicit POSIX-separator assertion. Skipped sidecar persistence of content_type/metadata: no current caller reads those fields from this backend; can be added if a real use case appears.
|
ziyanwan
approved these changes
Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of this change?
Unblocks the enterprise eval pipeline's per-example activity diagram by exposing two reusable primitives from community SAM:
Without these two additions, enterprise either has to fork/duplicate community mapping logic or require every developer to spin up MinIO before they can use eval features that rely on persisted artifacts.
How was this change implemented?
Public visualization mapper utility
src/solace_agent_mesh/gateway/http_sse/visualization_mapper.pyexporting the pure functioninfer_visualization_event_details(topic, payload, log_identifier=""). Body lifted verbatim fromWebUIBackendComponent._infer_visualization_event_details; the only change isself.log_identifierbecomes a parameter.src/solace_agent_mesh/gateway/http_sse/component.py— the existing method becomes a one-line delegating wrapper. Zero behavior change for chat.Filesystem object-storage backend
src/solace_agent_mesh/services/platform/storage/filesystem_client.py—FileSystemStorageClientimplementingObjectStorageClientagainst a local directory. One file per object ({root}/{bucket}/{key}).get_public_url/generate_presigned_urlreturnfile://URIs.src/solace_agent_mesh/services/platform/storage/factory.py— adds"filesystem"/"fs"/"local"aliases. Default is still"s3"; existing callers are unaffected.Key Design Decisions
{topic, payload}and rebuild on the frontend. Keeping mapping logic in one place (Python) avoids drift between a backend implementation and a TS port. The persisted shape matches what chat already produces, so the visualizer code path is shared between live and replay.EvalStorageServiceround-trips bytes through theObjectStorageClientinterface; a filesystem implementation is the lowest-friction way to make the dev experience match production semantics without requiring MinIO. Real disk also makes failure-mode testing (size limits, permissions, prefix delete) realistic.tests/unit/shared/storage/test_object_storage.pyimportsazure.storage.blobat module top, so it requires the optional Azure SDK to even collect. Filesystem tests live in their own module so they run in any minimal env.How was this change tested?
tests/unit/gateway/http_sse/test_visualization_mapper.py— 7 tests pinning the standalone function's contract (system events, topic-fallback for request/response/status, payload-summary truncation, serialization-failure handling).tests/unit/shared/storage/test_filesystem_storage.py— 11 tests exercising the real disk path: round-trip put/get, factory aliases (filesystem/fs/local/FILESYSTEM), delete-prefix subtree behavior, missing-keyStorageNotFoundError,list_objects, andfile://public URLs.~/.sam-data/object-storage/sam-eval-data/...); confirmed the per-example activity diagram renders the same flow chart the chat panel produces for the same agent.WebUIBackendComponentcontinue to pass.Is there anything the reviewers should focus on/be aware of?
component.pydiff is a behavior-preserving extraction. The 178-line removal is the original mapper body; the addition is a one-linereturn infer_visualization_event_details(topic, payload, self.log_identifier). Worth a quick eyeball to confirm no logic was dropped.FileSystemStorageClient.delete_prefixhas three code paths (target is dir / file / key-prefix string match). The third branch covers the case where the prefix doesn't correspond to a real directory but is a true key-substring — matches the behavior real S3 clients implement. All three are exercised by the new tests.generate_presigned_urlsilently ignoresexpires_in. Filesystem can't honor expiry; this is documented as a dev-only backend.put_objectmetadatais accepted and silently dropped (S3/Azure persist it). Acceptable for a dev backend; flagged here in case reviewers want a sidecar metadata file added before merge.