docs(architecture): describe the tree that exists - #1855
Merged
Conversation
`docs/architecture/` is governed by "current shipped state only" and had drifted behind three deletions — the plugin ABI and its cdylibs, the Deno SDK and the subprocess-polyglot machinery, and the schema/manifest layer. The docs still instructed a reader to depend on crates, call methods and read example directories that are not there. Audited each doc against the tree, one auditor per file, with a second pass whose job was to refute every "this is gone" verdict — a false one deletes documentation that is still correct. 72 candidate findings, 16 refuted, 56 applied. What was wrong, by kind: - **Crates that do not exist.** `streamlib-python-native` and `streamlib-deno-native` appear in `cargo tree -p` assertions a reader is told to run; the `streamlib-adapter-<name>-helpers` convention describes a crate shape the tree has none of. - **Methods and constants that do not exist.** `release_for_cross_process` is documented across 46 lines with Python and Deno call sites and a delegating SDK wrapper; nothing defines it. `STREAMLIB_ADAPTER_ABI_VERSION` has a section explaining its bump policy; it is nowhere in the tree. - **Deleted example directories cited as the canonical implementation.** Four `examples/polyglot-*/runner/` references; `examples/*/runner` matches nothing. - **Deno as a first-class target.** TypeScript code samples and "both Python AND Deno together" coverage rules, in a repo where Python is the only authoring runtime. - **Paths that moved.** `sdk/streamlib-python/` (now the wheel), `../streamlib-consumer-rhi` (lives under `runtime/`), `escalate_request.yaml` (the escalate wire is hand-written serde now). - **Six dead `CLAUDE.md#` anchors**, four of them in the banner every doc opens with — pointing at an "editing markdown documentation" section CLAUDE.md has never had. `#non-negotiables` is real and survives. Deletions carry the one-line marker `docs-policy.md` asks for, naming what went and why. Where a passage was merely wrong rather than dead it was corrected in place, in the doc's own voice, rather than removed. Six of the thirteen docs needed no content change: the RHI, kernel and texture docs describe machinery the pivot never touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughArchitecture documentation now describes the shipped Python-only runtime, consolidated adapter crates, helper-process IPC, current Vulkan integration, and updated texture and backend contracts. ChangesArchitecture documentation alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture/adapter-authoring.md (1)
93-114: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winApply the one-crate model to all authoring examples.
The checklist now defines an in-crate
[[bin]], but later text still namesstreamlib-adapter-<name>-helpers, starts the Metal walkthrough with three crates, and usescdylib-side terminology. Replace those references with the shipped helper-process and one-crate model.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture/adapter-authoring.md` around lines 93 - 114, Update all later authoring examples and walkthroughs to consistently use the single `streamlib-adapter-<name>` crate with its in-crate `[[bin]]` subprocess helper. Remove references to `streamlib-adapter-<name>-helpers`, revise the Metal walkthrough from three crates to one crate, and replace cdylib-specific terminology with the shipped helper-process model while preserving the dependency-boundary guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/adapter-runtime-integration.md`:
- Around line 141-144: Update the architecture diagram around the “streamlib
Python surface” and “streamlib._engine” layers so it no longer implies that
streamlib-adapter-skia is a wheel dependency; represent Skia as host-side
composition or list only adapters wired into the subprocess, consistent with
subprocess-rhi-parity.md.
- Around line 3-8: Limit the architecture documentation to shipped state by
removing or relocating roadmap and historical material. In
docs/architecture/adapter-runtime-integration.md lines 3-8, remove the
OpenGL/Skia migration wording; lines 125-132, remove the Deno sample removal
note; and lines 461-466, remove the install_default history note. In
docs/architecture/adapter-authoring.md lines 559-564 and 620-623, delete the
history notes, and at lines 781-784 move or remove the hypothetical Metal
walkthrough. In docs/architecture/texture-ring.md lines 259-261, remove the
VADR-TS-002 reference; in docs/architecture/third-party-gpu-backends.md lines
205-207, move the future engine-tier migration guidance to docs/plan/.
In `@docs/architecture/subprocess-rhi-parity.md`:
- Around line 92-109: Qualify the subsequent consumer-RHI import and
ConsumerVulkanDevice instantiation bullet so it applies only to Vulkan-device
adapters. Preserve the separate streamlib-adapter-opengl behavior, which uses
the EGL DMA-BUF path and does not use ConsumerVulkanTexture,
ConsumerVulkanBuffer, or ConsumerVulkanDevice.
- Around line 215-232: Update the documentation to consistently describe shipped
Python helper-process terminology instead of removed cdylib artifacts: replace
the cdylib labels in docs/architecture/subprocess-rhi-parity.md at lines 204 and
280 (anchor context lines 215-232), “Cdylibs use HostSurfaceRegistration” in
docs/architecture/texture-registration.md at lines 447-450 (site context lines
393-399), and the cdylib anti-pattern in
docs/architecture/third-party-gpu-backends.md at lines 231-236 (site context
line 140). Preserve the surrounding architecture guidance while using the
helper-process terminology at each site.
In `@docs/architecture/texture-registration.md`:
- Around line 30-31: Complete the obsolete CLAUDE.md reference migration: in
docs/architecture/texture-registration.md lines 30-31, link to
.claude/rules/engine-doctrine.md instead; update the remaining CLAUDE.md
reference in docs/architecture/adapter-authoring.md lines 24-25; and replace the
../../CLAUDE.md link in docs/architecture/texture-ring.md lines 259-261 with the
current .claude/rules/ doctrine reference.
---
Outside diff comments:
In `@docs/architecture/adapter-authoring.md`:
- Around line 93-114: Update all later authoring examples and walkthroughs to
consistently use the single `streamlib-adapter-<name>` crate with its in-crate
`[[bin]]` subprocess helper. Remove references to
`streamlib-adapter-<name>-helpers`, revise the Metal walkthrough from three
crates to one crate, and replace cdylib-specific terminology with the shipped
helper-process model while preserving the dependency-boundary guidance.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e4af327-3d49-42f5-876f-47666d3707c3
📒 Files selected for processing (10)
docs/architecture/adapter-authoring.mddocs/architecture/adapter-runtime-integration.mddocs/architecture/adapter-timeline-single-writer.mddocs/architecture/ray-tracing-kernel.mddocs/architecture/subprocess-rhi-parity.mddocs/architecture/surface-adapter.mddocs/architecture/texture-registration.mddocs/architecture/texture-ring.mddocs/architecture/third-party-gpu-backends.mddocs/architecture/vendored-vulkanalia.md
Comment on lines
+215
to
+232
| │ section); its helper-process customers reach it through the │ | ||
| │ wrapped adapter, and it is not a dep of the wheel. │ | ||
| └───────┼──────────────────────────────────────────────────────────────┘ | ||
| ▼ | ||
| ┌──────────────────────┐ ┌──────────────────────┐ | ||
| │ PYTHON SUBPROC │ │ DENO SUBPROC │ | ||
| │ Cargo: consumer-rhi │ │ Cargo: consumer-rhi │ | ||
| │ + adapter-{abi, │ │ + adapter-{abi, │ | ||
| │ vulkan, opengl, │ │ vulkan, opengl, │ | ||
| │ cpu-readback, │ │ cpu-readback, │ | ||
| │ cuda}; │ │ cuda}; │ | ||
| │ NOT full streamlib │ │ NOT full streamlib │ | ||
| └──────────────────────┘ └──────────────────────┘ | ||
| ┌──────────────────────┐ | ||
| │ PYTHON HELPER │ | ||
| │ Imports the wheel: │ | ||
| │ consumer-rhi + │ | ||
| │ surface-client + │ | ||
| │ adapter-cuda; │ | ||
| │ import-side Vulkan │ | ||
| │ only │ | ||
| └──────────────────────┘ | ||
| ``` | ||
|
|
||
| `cargo tree -p streamlib-{python,deno}-native | grep -c "^streamlib v"` | ||
| returns 0 — the capability boundary is enforced by Cargo dep resolution | ||
| itself. | ||
| `cargo xtask check-boundaries` fails if `streamlib` appears outside | ||
| `[dev-dependencies]` in any adapter crate's manifest — the capability | ||
| boundary is enforced by Cargo dep resolution itself. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use the shipped Python helper-process terminology consistently.
The current pages still describe removed cdylib artifacts in surrounding sections.
docs/architecture/subprocess-rhi-parity.md#L215-L232: replace thecdyliblabels at Lines [204] and [280].docs/architecture/texture-registration.md#L393-L399: replace “Cdylibs useHostSurfaceRegistration” at Lines [447-450].docs/architecture/third-party-gpu-backends.md#L140-L140: replace the cdylib anti-pattern at Lines [231-236].
📍 Affects 3 files
docs/architecture/subprocess-rhi-parity.md#L215-L232(this comment)docs/architecture/texture-registration.md#L393-L399docs/architecture/third-party-gpu-backends.md#L140-L140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/subprocess-rhi-parity.md` around lines 215 - 232, Update
the documentation to consistently describe shipped Python helper-process
terminology instead of removed cdylib artifacts: replace the cdylib labels in
docs/architecture/subprocess-rhi-parity.md at lines 204 and 280 (anchor context
lines 215-232), “Cdylibs use HostSurfaceRegistration” in
docs/architecture/texture-registration.md at lines 447-450 (site context lines
393-399), and the cdylib anti-pattern in
docs/architecture/third-party-gpu-backends.md at lines 231-236 (site context
line 140). Preserve the surrounding architecture guidance while using the
helper-process terminology at each site.
Comment on lines
+30
to
+31
| [CLAUDE.md "Engine-wide defects get fixed at the engine | ||
| layer"](../../CLAUDE.md#non-negotiables)) makes |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the obsolete CLAUDE.md reference migration.
The current doctrine lives under .claude/rules/, but several changed sections still leave readers on the old root-level document.
docs/architecture/texture-registration.md#L30-L31: replace../../CLAUDE.md#non-negotiableswith.claude/rules/engine-doctrine.md.docs/architecture/adapter-authoring.md#L24-L25: update the remainingCLAUDE.mdreference at Line [503].docs/architecture/texture-ring.md#L259-L261: replace the remaining../../CLAUDE.mdlink at Lines [290-291].
📍 Affects 3 files
docs/architecture/texture-registration.md#L30-L31(this comment)docs/architecture/adapter-authoring.md#L24-L25docs/architecture/texture-ring.md#L259-L261
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/architecture/texture-registration.md` around lines 30 - 31, Complete the
obsolete CLAUDE.md reference migration: in
docs/architecture/texture-registration.md lines 30-31, link to
.claude/rules/engine-doctrine.md instead; update the remaining CLAUDE.md
reference in docs/architecture/adapter-authoring.md lines 24-25; and replace the
../../CLAUDE.md link in docs/architecture/texture-ring.md lines 259-261 with the
current .claude/rules/ doctrine reference.
…hipped design Three findings from review, all verified against the tree. - The layered-architecture diagram listed `streamlib-adapter-skia` as statically linked into the wheel. The wheel's Cargo.toml does not name it and §Distribution says "the wheel's adapter closure excludes skia". The diagram also declared every adapter generic over `D: VulkanRhiDevice`, which the same PR had just corrected elsewhere. - `subprocess-rhi-parity.md` contradicted itself after my own edit: one bullet said `streamlib-adapter-opengl` is not generic over the device flavor, the next said every subprocess instantiates against a consumer `VkDevice`. The subprocess bullet is now scoped to the Vulkan-device adapters, with opengl's EGL path named. - Docs policy bans tracker references and proposed work outright. Removed: three "will migrate in a separate issue" clauses, a `VADR-TS-002` reference, a paragraph promising a same-PR migration of two libraries, and the 70-line "Hypothetical walkthrough — Metal on macOS via MoltenVK" whose own text calls the adapter "not yet shipped". The removal markers this PR adds are NOT removed. Review read them as "history of superseded designs", but `docs-policy.md` mandates them in the same breath as the ban: "outright deletion is allowed when content is provably wrong — leave a one-line marker saying what was removed and why." The rule contradicts itself; following the explicit instruction is the defensible read, and changing it is a rules change rather than a doc fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Worked all three. Two were real errors of fact, one is a rule that contradicts itself. Skia in the wheel — valid, fixed. The diagram listed The consumer-RHI bullet — valid, and it was my regression. My edit added "… Roadmap / trackers / unshipped design — valid, fixed. Docs policy bans these outright ("no tracker references, no dates, no roadmap / proposed-work"). Removed: three "will migrate in a separate issue" clauses, the The removal markers stay — skipping four of the eight sites. Review read them as "history of superseded designs", which the policy does ban. But the very next bullet of Verification: 0 broken relative links, |
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
1 participant
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/architecture/is governed by "current shipped state only — no roadmap, no history of superseded designs" (.claude/rules/docs-policy.md). It had drifted behind three deletions: the plugin ABI and its cdylibs (#1715), the Deno SDK and subprocess-polyglot machinery (#1715), and the schema/manifest layer (#1813 / #1854).The result was docs instructing a reader to depend on crates, call methods and read example directories that are not in the tree.
10 files, +205 / −254.
Method
One auditor per stale doc, then a second agent per doc whose only job was to refute every "this is gone" verdict. A false dead-verdict deletes documentation that is still correct, which is the expensive error here — so the refuter was told to default to refuting when uncertain.
72 candidate findings → 16 refuted → 56 applied. The 22% refutation rate is the point: those 16 would have been wrong edits. Examples of what the refuters saved —
subprocess-rhi-parity.md:174-175,adapter-runtime-integration.md:300-308,texture-registration.md:359-361,ray-tracing-kernel.md:58-62, all of which describe live machinery that merely reads like the deleted model.What was wrong, by kind
cargo tree -p streamlib-{python,deno}-native— an assertion the reader is told to run, naming two deleted packages. Thestreamlib-adapter-<name>-helpersconvention describes a crate shape the tree has none of.release_for_cross_processdocumented across 46 lines with Python and Deno call sites and a delegating SDK wrapper — nothing defines it.STREAMLIB_ADAPTER_ABI_VERSIONhad a section explaining its bump policy; it is nowhere.examples/polyglot-*/runner/references;examples/*/runnermatches nothing.sdk/streamlib-python/(now the wheel),../streamlib-consumer-rhi(lives underruntime/),escalate_request.yaml(the wire is hand-written serde now).CLAUDE.md#anchorsDeletions carry the one-line marker
docs-policy.mdasks for, naming what went and why. Where a passage was merely wrong rather than dead it was corrected in place, in the doc's own voice.Verification
](…)target resolves.CLAUDE.md#anchor checked against CLAUDE.md's real headings.#non-negotiablesis genuine and survives; the other six are fixed.vendored-vulkanalia.mdthat are relative to the vendored crate root, which the surrounding sentence scopes.xtask check-no-in-process-placement(scans markdown)xtask check-boundariesship-change-removed-gatetestsNo code touched —
docs/architecture/only.Notes for owner
1. Six of thirteen docs needed no content change —
compute-kernel,graphics-kernel,ray-tracing-kernel,texture-readback,texture-ring,adapter-timeline-single-writer. They describe RHI/GPU machinery the pivot never touched. This was localized rot, not a rotten directory.2. One adjacent section left standing, deliberately.
adapter-authoring.md's### Why not add a Vulkan device handle to the producer adapteris rejected-alternative rationale forrelease_for_cross_process— a method that does not exist. It was outside the verified range so I did not extend the deletion into it. Two questions for you: rationale for a nonexistent API is arguably dead, anddocs-policy.mdputs "why this over the alternatives" indocs/decisions/rather thandocs/architecture/anyway. Candidate for relocation or removal.3. The banner said "Living document. Validate, update, and critique freely." That reads as an invitation to speculate, which is how a doc drifts from shipped state. It now states the constraint instead. If you want the invitation back, it belongs somewhere that is not the doc it licenses editing.
🤖 Generated with Claude Code
Summary by CodeRabbit