docs: add SDK package boundary and dependency direction map - #328
Merged
El-swaggerito merged 1 commit intoJul 27, 2026
Merged
Conversation
docs/dependency_direction_map.md was a 4-line stub referencing "core, utils, and contract clients" — none of which match the SDK's actual module names. Replaced it with a real map, verified against src/'s actual imports (grep -rhoE "from '\.\./[a-z]+" per module, not inferred from names): - Full per-module ownership/dependency table for all 12 real src/ directories. architecture.md's existing layer diagram only covered 9 of them - account, errors, and vault existed in src/ with no documentation at all anywhere in the repo. - account is Layer 1 infrastructure (an identity/signing abstraction with a pluggable Signer + hardware/mobile signer extension point), not a peer feature module, even though only transactions has adopted it so far - that's the intended migration direction, not a violation of the "features don't cross-import" rule. - vault has zero implementation of its own: it's a pure re-export facade over soroban, existing only for a more discoverable public import path. - config and diagnostics reference each other, which looks like exactly the cycle the layer rule forbids. It isn't one: diagnostics/hooks.ts (a leaf config depends on) and diagnostics/report.ts (which depends on config) never import each other, so npm run check:circular reports zero cycles despite the module-level mutual reference. Documented precisely since the circular-dependency checker operates on individual files, not directories, and a future import into the wrong file could create a real cycle it wouldn't catch as clearly. - Security-sensitive boundaries called out explicitly: account/wallet are the only modules touching secret key material, errors owns redaction, diagnostics has its own independent redaction layer. - Correct vs. incorrect cross-module and package-root import examples. Also added the account, errors, and vault module sections to architecture.md (present in src/, undocumented there) and pointed its now-superseded hand-drawn layer diagram at the new, verified map instead of maintaining two potentially-drifting versions of the same DAG. Neither new-doc file previously existed as real content and neither is linked from README.md before this commit - added both. Closes Axionvera#285
6 tasks
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.
Summary
docs/dependency_direction_map.mdexisted only as a 4-line stub — a title and one sentence referencing "core, utils, and contract clients," none of which match any real module in this SDK. This replaces it with a real map, verified directly againstsrc/'s actual imports rather than inferred from module names or copied fromarchitecture.md's existing (partially stale) diagram.Per the acceptance criteria in #285:
src/directories.architecture.md's existing layer diagram only documented 9 of them —account,errors, andvaultexist insrc/with real, substantial implementations and were undocumented anywhere in the repo.architecture.mdin its existing format, plus the full ownership table in the new map.accountis correctly placed in the infrastructure layer (it's an identity/signing abstraction with a pluggableSigner+ hardware/mobile-signer extension point), not a peer feature module — even though onlytransactionshas adopted it so far.vaultis documented as a pure re-export facade oversorobanwith no implementation of its own. And theconfig↔diagnosticsmutual reference — which looks exactly like the cycle the layer rule forbids — is explained precisely:diagnostics/hooks.ts(a leafconfigdepends on) anddiagnostics/report.ts(which depends onconfig) never import each other, sonpm run check:circularreports zero cycles despite the module-level mutual reference. Worth knowing before adding a new import between the two, since the circular-dependency checker operates on individual files, not directories.account/walletare the only modules that ever touch secret key material,errorsowns redaction,diagnosticshas its own independent redaction layer.accountabstraction.README.md's Documentation section before this PR —architecture.md(already large and useful) was effectively undiscoverable.Pre-existing, out-of-scope finding
While verifying the repo before touching docs,
npm run verifyreported 46 pre-existing test failures across 5 files (retry-policy.test.ts,types/asset.test.ts, and others) — functional bugs in retry classification and asset validation, unrelated to module structure.npm run lint(tsc --noEmit) andnpm run check:circularboth pass cleanly, which is what this doc's claims actually depend on, so these failures don't block anything in this PR. Flagging for visibility rather than expanding this PR's scope into unrelated bug fixes.Test plan
npm run lint(tsc --noEmit) — passesnpm run check:circular— passes, 46 modules, 0 cyclesgrep -rhoE "from '\.\./[a-z]+" src/<module>per module, not assumedarchitecture.mdand the new dependency mapCloses #285