docs: add transport SDK cutover examples#103
Open
cryptoxdog wants to merge 2 commits into
Open
Conversation
Add a GMP report and the associated build plan as reference material for future SDK cutover work in this repo. Made-with: Cursor
|
📋 Best Practices for Large Changes
✅ This PR meets minimum requirements but is larger than recommended. |
📝 WalkthroughWalkthroughThis change adds two new documentation files detailing a comprehensive transport-runtime cutover plan from a local Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09b1be0e66
ℹ️ 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".
| ## Current State Summary | ||
|
|
||
| - The original plan understated the scope. `Gate_SDK` is a full node-runtime repo, not just a transport helper package. Its public API includes `create_node_app`, runtime execution/registration, transport models, security validation, `GateClient`, and orchestrator helpers. | ||
| - Engine business logic still already sits behind handler functions in [app/engines/handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/handlers.py), which remains the right long-term execution surface. |
There was a problem hiding this comment.
This plan hardcodes links to /Users/ib-mac/..., which only resolve on one developer machine and become broken links for everyone else (for example in GitHub rendering or local clones), making the migration references difficult to verify and maintain. Please switch these references to repository-relative links (or plain code paths) so the document is navigable in shared environments.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
reports/transport-sdk_cutover_b23e837c.plan.md (1)
207-207: Minor style: vary sentence openings.Three consecutive sentences begin with "Update". Consider rephrasing for better readability:
-Update architecture and governance references that describe `chassis/` as an in-repo transport library. -Update contract language from PacketEnvelope to `TransportPacket` where the SDK becomes source of truth. -Update docs and config that assume direct peer node URLs, especially `graph_node_url`-style routing or generic node clients. +Update architecture and governance references that describe `chassis/` as an in-repo transport library. +Revise contract language from PacketEnvelope to `TransportPacket` where the SDK becomes source of truth. +Refresh docs and config that assume direct peer node URLs, especially `graph_node_url`-style routing or generic node clients.As per coding guidelines flagged by static analysis tools: three successive sentences beginning with the same word should be reworded for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@reports/transport-sdk_cutover_b23e837c.plan.md` at line 207, The sentence "Update docs and config that assume direct peer node URLs, especially `graph_node_url`-style routing or generic node clients." repeats the verb "Update" as part of three consecutive sentences; reword this sentence to vary its opening and improve flow—for example, start with "Revise" or "Adjust" and mention `graph_node_url`-style routing and generic node clients so the meaning stays the same; ensure the other two nearby sentences are not all starting with "Update" to avoid three consecutive identical openings.reports/GMP-Report-TRANSPORT-SDK-Cutover.md (1)
71-81: Clarify validation scope and SDK dependency confirmation.The validation section shows pytest commands that passed, but doesn't demonstrate that those tests actually exercise SDK-backed runtime behavior rather than the legacy
chassis/implementation.The installation command on line 72 installs from
Gate_SDKrepository, but the tests listed (lines 76, 80) may have passed because they still use the old code paths.Consider adding validation commands that explicitly confirm SDK integration, such as:
- Checking that
constellation_node_sdkimports succeed- Verifying that
create_node_appis actually used inapp/main.py- Confirming
GateClientorTransportPacketare imported and used🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@reports/GMP-Report-TRANSPORT-SDK-Cutover.md` around lines 71 - 81, Add explicit SDK integration checks to the validation section: run a quick import probe for constellation_node_sdk to ensure the installed Gate_SDK package is importable, grep the project for create_node_app to confirm the SDK factory is referenced instead of legacy chassis entry points, and verify GateClient and TransportPacket are actually imported/used by the runtime entry (search for those symbols in the app bootstrap and tests). Also include a runtime smoke check that starts the app bootstrap (the module that wires node creation) and asserts it constructs the SDK node (e.g., ensure the created app object is an instance produced by create_node_app) so test results demonstrate SDK-backed behavior rather than legacy chassis usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@reports/GMP-Report-TRANSPORT-SDK-Cutover.md`:
- Around line 85-88: Phase 5 text incorrectly claims use of
constellation_node_sdk.runtime.handlers.register_handler and
GateClient/TransportPacket; instead update the Phase 5 verification to reflect
the actual codebase: state that active handler registration is done via the
_engine_handlers dict (used in chassis/actions.py) and that there is no
GateClient or TransportPacket usage present, or alternatively mark those two
items explicitly as "Target state" (post-completion goals) rather than verified
implementation; adjust the bullet points to either replace the SDK/GateClient
claims with the dict-based handler and absence of GateClient, or prepend "Target
state:" to the SDK handler and GateClient/TransportPacket bullets.
In `@reports/transport-sdk_cutover_b23e837c.plan.md`:
- Around line 47-83: The doc contains hardcoded absolute paths (e.g.,
/Users/ib-mac/Dropbox/.../app/engines/handlers.py) which must be replaced with
repository-relative paths (e.g., app/engines/handlers.py) throughout the
"Current State Summary" and "Precise File Fate Matrix" (notably the listed
entries for chassis/envelope.py, app/engines/handlers.py,
app/api/v1/chassis_endpoint.py, app/main.py, app/engines/orchestration_layer.py,
app/engines/graph_sync_client.py, app/services/chassis_handlers.py,
app/services/graph_return_channel.py, and all other chassis/* and app/*
mentions); update every occurrence in the ranges called out (lines ~47-83,
109-153, 157-218) so no absolute filesystem paths or usernames remain and ensure
formatting uses repo-root relative paths consistently.
---
Nitpick comments:
In `@reports/GMP-Report-TRANSPORT-SDK-Cutover.md`:
- Around line 71-81: Add explicit SDK integration checks to the validation
section: run a quick import probe for constellation_node_sdk to ensure the
installed Gate_SDK package is importable, grep the project for create_node_app
to confirm the SDK factory is referenced instead of legacy chassis entry points,
and verify GateClient and TransportPacket are actually imported/used by the
runtime entry (search for those symbols in the app bootstrap and tests). Also
include a runtime smoke check that starts the app bootstrap (the module that
wires node creation) and asserts it constructs the SDK node (e.g., ensure the
created app object is an instance produced by create_node_app) so test results
demonstrate SDK-backed behavior rather than legacy chassis usage.
In `@reports/transport-sdk_cutover_b23e837c.plan.md`:
- Line 207: The sentence "Update docs and config that assume direct peer node
URLs, especially `graph_node_url`-style routing or generic node clients."
repeats the verb "Update" as part of three consecutive sentences; reword this
sentence to vary its opening and improve flow—for example, start with "Revise"
or "Adjust" and mention `graph_node_url`-style routing and generic node clients
so the meaning stays the same; ensure the other two nearby sentences are not all
starting with "Update" to avoid three consecutive identical openings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0286d802-5c0e-4477-9db3-dbbf16214aad
📒 Files selected for processing (2)
reports/GMP-Report-TRANSPORT-SDK-Cutover.mdreports/transport-sdk_cutover_b23e837c.plan.md
Comment on lines
+85
to
+88
| - Runtime ingress now comes from `create_node_app(...)` in `app/main.py`. | ||
| - Active handler registration uses `constellation_node_sdk.runtime.handlers.register_handler`. | ||
| - Active outbound transport uses `GateClient`/`TransportPacket`; direct peer `httpx` transport was removed from the live runtime helpers. | ||
| - Packet/runtime tests now validate SDK behavior instead of the deleted local chassis internals. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check actual handler registration patterns in the codebase
# Search for SDK handler registration calls
echo "=== Searching for SDK register_handler usage ==="
rg -n 'constellation_node_sdk.*register_handler|from constellation_node_sdk.*import.*register_handler' -g '!reports/' -A2
# Search for legacy _engine_handlers dict usage
echo -e "\n=== Searching for _engine_handlers dict usage ==="
rg -n '_engine_handlers\s*=' -g '!reports/' -A2
# Search for GateClient usage
echo -e "\n=== Searching for GateClient usage ==="
rg -n 'GateClient|from.*constellation_node_sdk.*import.*GateClient' -g '!reports/' -A2Repository: cryptoxdog/Cognitive.Engine.Graphs
Length of output: 546
Correct Phase 5 claims to match actual codebase.
The Phase 5 verification section (lines 85-88) states that "Active handler registration uses constellation_node_sdk.runtime.handlers.register_handler" and "Active outbound transport uses GateClient/TransportPacket", but codebase verification shows:
- No SDK handler registration found: Zero occurrences of
constellation_node_sdk.runtime.handlers.register_handleranywhere in the codebase. Actual implementation uses direct dict assignment:_engine_handlers = {}inchassis/actions.py:51. - No GateClient usage found: Zero occurrences of
GateClientimport or instantiation.
Given the PARTIAL-VALIDATED status, either:
- Update the Phase 5 section to describe the actual current state (dict-based handlers, no GateClient), or
- Clearly mark these as Target state (post-completion goals) rather than verified implementation
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reports/GMP-Report-TRANSPORT-SDK-Cutover.md` around lines 85 - 88, Phase 5
text incorrectly claims use of
constellation_node_sdk.runtime.handlers.register_handler and
GateClient/TransportPacket; instead update the Phase 5 verification to reflect
the actual codebase: state that active handler registration is done via the
_engine_handlers dict (used in chassis/actions.py) and that there is no
GateClient or TransportPacket usage present, or alternatively mark those two
items explicitly as "Target state" (post-completion goals) rather than verified
implementation; adjust the bullet points to either replace the SDK/GateClient
claims with the dict-based handler and absence of GateClient, or prepend "Target
state:" to the SDK handler and GateClient/TransportPacket bullets.
Comment on lines
+47
to
+83
| - Engine business logic still already sits behind handler functions in [app/engines/handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/handlers.py), which remains the right long-term execution surface. | ||
| - There are two PacketEnvelope implementations today: [chassis/envelope.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/envelope.py) and [app/engines/chassis_contract.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/chassis_contract.py). The SDK cutover should remove both as first-class implementations. | ||
| - There are more transport touchpoints than the first plan captured: | ||
| - inbound edge in [app/api/v1/chassis_endpoint.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/api/v1/chassis_endpoint.py) | ||
| - bootstrap in [app/main.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/main.py) | ||
| - handler registration in [app/engines/orchestration_layer.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/orchestration_layer.py) | ||
| - direct node dispatch in [app/engines/graph_sync_client.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/graph_sync_client.py) and [app/engines/packet_router.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/packet_router.py) | ||
| - a second local handler-registry path in [app/services/chassis_handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/services/chassis_handlers.py) | ||
| - graph return-channel docs and registration assumptions in [app/services/graph_return_channel.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/services/graph_return_channel.py) | ||
| - The “parallel registries” problem is worse than duplication alone: [app/services/chassis_handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/services/chassis_handlers.py) defines `register_all_handlers()`, but that path is not wired from [app/main.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/main.py) or [app/engines/orchestration_layer.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/orchestration_layer.py), so some packet actions appear to be dead rather than merely duplicated. | ||
| - Current outbound wire formats are inconsistent. [app/engines/graph_sync_client.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/graph_sync_client.py) builds one PacketEnvelope-like shape, [app/engines/packet_router.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/packet_router.py) builds another, and [chassis/router.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/router.py) expects its own ingress semantics. The revised plan must converge these to one canonical TransportPacket flow. | ||
|
|
||
| ## Precise File Fate Matrix | ||
|
|
||
| - Remove and replace with SDK runtime or SDK imports: | ||
| - [chassis/__init__.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/__init__.py) | ||
| - [chassis/router.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/router.py) | ||
| - [chassis/registry.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/registry.py) | ||
| - [chassis/envelope.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/envelope.py) | ||
| - [chassis/node_client.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/node_client.py) | ||
| - [chassis/lifecycle.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/lifecycle.py) | ||
| - Replace with SDK runtime ownership or reduce to service-only composition: | ||
| - [app/api/v1/chassis_endpoint.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/api/v1/chassis_endpoint.py): no longer assume this stays. Under the revised default, `/v1/execute` should come from `create_node_app`; this file either disappears or shrinks to non-SDK extras like `/v1/outcomes` if that route remains app-specific. | ||
| - [app/main.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/main.py): newly in scope. The plan must decide whether the base `FastAPI` app is replaced by SDK app creation and then augmented with service-specific routers, middleware, and lifespan hooks. | ||
| - Keep, but rewire through the SDK: | ||
| - [app/engines/orchestration_layer.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/orchestration_layer.py): keep orchestration behavior, replace local handler registration, and remove assumptions about direct peer routing. | ||
| - [app/services/graph_return_channel.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/services/graph_return_channel.py): keep domain behavior, but retarget any chassis registration language or wiring to SDK runtime registration if still needed. | ||
| - Keep as engine-local business logic: | ||
| - [app/engines/handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/handlers.py) | ||
| - `result_store`, convergence/enrichment services, KB resolution, and other domain logic outside transport. | ||
| - Keep only if re-expressed in Gate-only terms; otherwise remove or redesign: | ||
| - [app/engines/graph_sync_client.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/graph_sync_client.py): current direct `graph_url` + `/v1/execute` model conflicts with the SDK’s Gate-only outbound rule and must be replaced or fundamentally redesigned around `GateClient`. | ||
| - [app/engines/packet_router.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/packet_router.py): current peer URL routing and fire-and-forget dispatch are in direct tension with the SDK architecture and need a dedicated migration step. | ||
| - Collapse or delete after choosing the canonical SDK surface: | ||
| - [app/engines/chassis_contract.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/chassis_contract.py): prefer deletion or a very short compatibility facade while TransportPacket migration is in flight. | ||
| - [app/services/chassis_handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/services/chassis_handlers.py): likely remove or merge, because it is a second in-repo handler registry that duplicates runtime responsibilities and may currently be unwired dead code. | ||
| - [chassis/middleware.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/middleware.py): likely remove; the SDK runtime already owns execution, readiness, and observability concerns. |
There was a problem hiding this comment.
Remove hardcoded local filesystem paths from documentation.
The "Current State Summary" and "Precise File Fate Matrix" sections contain absolute filesystem paths from a local development machine:
/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/...
These hardcoded paths should not be committed to version control because:
- They expose local machine username and directory structure
- They break for any other developer or CI environment
- They make the documentation non-portable
Replace all absolute paths with relative paths from the repository root (e.g., app/engines/handlers.py instead of /Users/ib-mac/Dropbox/.../app/engines/handlers.py).
🔧 Example fix for lines 47-58
-Engine business logic still already sits behind handler functions in [app/engines/handlers.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/handlers.py), which remains the right long-term execution surface.
+Engine business logic still already sits behind handler functions in `app/engines/handlers.py`, which remains the right long-term execution surface.
-There are two PacketEnvelope implementations today: [chassis/envelope.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/chassis/envelope.py) and [app/engines/chassis_contract.py](/Users/ib-mac/Dropbox/Repo_Dropbox_IB/Enrichment.Inference.Engine/app/engines/chassis_contract.py). The SDK cutover should remove both as first-class implementations.
+There are two PacketEnvelope implementations today: `chassis/envelope.py` and `app/engines/chassis_contract.py`. The SDK cutover should remove both as first-class implementations.Apply similar changes throughout lines 47-83, 109-153, and 157-218.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reports/transport-sdk_cutover_b23e837c.plan.md` around lines 47 - 83, The doc
contains hardcoded absolute paths (e.g.,
/Users/ib-mac/Dropbox/.../app/engines/handlers.py) which must be replaced with
repository-relative paths (e.g., app/engines/handlers.py) throughout the
"Current State Summary" and "Precise File Fate Matrix" (notably the listed
entries for chassis/envelope.py, app/engines/handlers.py,
app/api/v1/chassis_endpoint.py, app/main.py, app/engines/orchestration_layer.py,
app/engines/graph_sync_client.py, app/services/chassis_handlers.py,
app/services/graph_return_channel.py, and all other chassis/* and app/*
mentions); update every occurrence in the ranges called out (lines ~47-83,
109-153, 157-218) so no absolute filesystem paths or usernames remain and ensure
formatting uses repo-root relative paths consistently.
Collaborator
Author
PR #103 Review: docs: add transport SDK cutover examplesAuthor: cryptoxdog DescriptionSummary
Test plan
Made with Cursor Summary by CodeRabbit
CommitsChanged Files📝 Docs (2 files)
Automated AnalysisDiff Findings✅ No issues found in diff analysis Test CoverageTest coverage looks adequate for changed files. Local LintSkipped (repo not checked out locally or linters not found). Summary✅ LOOKS GOOD — No automated issues found. Ready for human review. Automated PR review • 2026-04-21 18:07 |
|
📋 Best Practices for Large Changes
✅ This PR meets minimum requirements but is larger than recommended. |
|
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
2 participants
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.
Summary
reports/locationTest plan
tests/integration/test_handlers.pyimport mismatch forDomainSpecLoader)Made with Cursor
Summary by CodeRabbit