spec §6: document the client screen contract + normative hot-path rules - #4
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR rewrites §6 of the plugin spec, expands the best-practices guide with client-screen and visualization rules, and updates the example README and changelog to match the new section structure and references. ChangesClient Surface Specification Rewrite
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
topkoa
force-pushed
the
docs/client-runtime-section
branch
from
July 6, 2026 01:11
712c7e7 to
5d4ca29
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@spec/plugin-spec-v1.md`:
- Around line 263-269: The re-hydration guidance currently uses a process-wide
singleton guard, which can block later screen instances. Update the wording
around the plugin `script` idempotency rule and the
`plugin-runtime-idempotent.v1` standard so the no-op check is scoped to a
mounted container or per-screen runtime token rather than a module-global
global, keeping the guidance consistent with the per-instance behavior described
in §6.4.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3572023d-2199-4fe4-abae-d07902e44ca0
📒 Files selected for processing (3)
CHANGELOG.mdexamples/full-plugin/README.mdspec/plugin-spec-v1.md
This was referenced Jul 6, 2026
There was a problem hiding this comment.
Pull request overview
This PR replaces the §6 “client runtime API is out of scope” placeholder with a structured client-screen contract: a normative, portable layer (idempotence + performance/resource-discipline rules) plus a descriptive, Host-versioned runtime-surface description, and updates docs/changelog references accordingly.
Changes:
- Rewrites
spec/plugin-spec-v1.md§6 to document screen mount lifecycle, activation/visibility signaling, the Host-provided runtime surface, and normative shared-main-thread performance rules. - Updates the full-plugin example README to link to the new §6 anchors and clarify which parts are normative vs Host-versioned.
- Adds an
[Unreleased]changelog entry summarizing the §6 rewrite and section renumbering.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spec/plugin-spec-v1.md | Replaces §6 placeholder with detailed screen lifecycle + runtime surface description and normative performance rules; renumbers §6.5–6.7. |
| examples/full-plugin/README.md | Updates spec links to new §6 structure (mount lifecycle, runtime surface, performance rules). |
| CHANGELOG.md | Documents the §6 rewrite and renumbering under [Unreleased]. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace the "client runtime API is out of scope" placeholder in §6 with a real, ground-truthed contract, split into a portable normative layer and a descriptive (Host-versioned, not-frozen) layer. - §6.1 Screen mount lifecycle: Host creates an id-derived container, sources its markup from `screen`, and executes `script` once. There is NO Host-invoked entry point — `script` self-executes and finds its own DOM. Normative: re-hydration MUST be idempotent (a second execution installs nothing twice); declaring plugin-runtime-idempotent.v1 asserts this. - §6.2 Screen activation and visibility: one screen active at a time; the Host signals transitions; plugins should react and suspend background work when hidden. - §6.3 The client runtime surface (descriptive, explicitly not frozen): the event bus, contribution registries, and the forward-stable capability control plane (capability-pipelines.v1); raw window globals documented as supported-but-legacy. No global Host version — per-object `version` sentinels, feature-detect. - §6.4 Performance and the shared main thread (NORMATIVE): SHOULD NOT do per-frame DOM queries / layout reads / IO; don't observe or mutate the shell (use contribution registries); suspend loops when hidden; keep state per-instance. This anchors the best-practices performance guidance in the contract. Renumber Settings/Styles/Static-assets to §6.5–6.7 and fix the §3 cross-ref. Update the full-plugin README pointer and CHANGELOG [Unreleased]. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: K. O. A. <topkoa@gmail.com>
…ctive-screen) - §6.1: drop misleading §6.4 cross-ref on identifier namespacing; namespace under id. - §6.1: clarify the re-hydration guard suppresses global side effects, distinct from per-instance screen state. - §6.4: spell out 'blocking I/O or network I/O' with a localStorage example. - §6.4: reword to 'mount more than one instance of a screen' so it doesn't contradict §6.2's one-active-screen statement. Signed-off-by: K. O. A. <topkoa@gmail.com>
topkoa
force-pushed
the
docs/client-runtime-section
branch
from
July 6, 2026 04:24
5d4ca29 to
145a383
Compare
Add a "Client screen & the shared main thread" section to the best-practices guide, grounded in real feedBack performance regressions rather than generic advice. The screen runs unsandboxed on the same main thread as a ~60fps note highway, and most historical plugin perf problems came from that. New rules: - Never do DOM queries / layout reads / style writes on a per-frame or high-frequency path (rAF, draw(), short setInterval, MutationObserver) — resolve refs once on mount; batch reads then writes. (A profiled lag report was three plugins each doing a per-frame querySelectorAll.) - Don't DOM-observe or mutate the app shell — especially the song/library cards. Contribute UI via the Host registration APIs; no subtree MutationObserver on shared containers; keep per-item predicates O(1). (Replaces the legacy .song-card DOM-observation pattern.) - No synchronous localStorage / await fetch on a hot or gameplay-event path. - Make re-hydration idempotent (plugin-runtime-idempotent.v1) with a stable window singleton; a second script run must be a no-op. - Stop rAF loops / event subscriptions when hidden; keep state per-instance (splitscreen runs multiple instances). - Reach other plugins via capability claim/dispatch/release, not their globals. Regroup the guide (Getting started / Server routes / Client screen / Shipping) and expand the pre-publish checklist with a client-performance block. Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: K. O. A. <topkoa@gmail.com>
Rule 11 + checklist: distinguish synchronous localStorage from awaited network I/O; fetch itself is asynchronous. Signed-off-by: K. O. A. <topkoa@gmail.com>
Add a "Visualizations" section to the best-practices guide for type: "visualization" plugins, ground-truthed against the Host's renderer contract and the recent splitscreen/settings fixes. - Factory pattern (rule 15): window.feedBackViz_<id> MUST be a factory function returning a fresh renderer per call — this is what makes splitscreen's N simultaneous panels work; a singleton shares one context/canvas/meshes across panels and collides. Includes the renderer interface (draw required; init/resize/destroy/contextType/readyPromise optional) and lifecycle. - Per-instance resources + destroy() cleanup (16); read-only per-frame bundle, allocation-free draw() (17); self-detect canvas size drift (18). - Settings communication (19): declare `settings` on the visualization capability and implement applySetting(key, value) on the instance — the Host applies each change to the specific per-panel instance (inherently per-panel). Documents the concrete failure modes recent fixes addressed: apply-live-never- reload, no cross-setting leakage (backfill once, don't mirror on read), deliberate per-panel vs global key scoping, fan-out to all panels, and the settings panel loading before the renderer. - Fail-safe auto-revert (20): guard draw() or the Host reverts to default after repeated throws. Renumber the Shipping section to 21-25 and add a Visualizations checklist block. Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: K. O. A. <topkoa@gmail.com>
… (rule 19) Rule 19 said "the Host owns persistence" but never addressed localStorage. Make it explicit: prefer Host-owned persistence (declare the setting, apply live, let the Host store/replay) and do not hand-roll settings into localStorage — that is what keeps export/import and backups whole and stops per-panel copies drifting. For a self-managed viz that predates the contract, add the quota-safe rule from the fixes: localStorage is synchronous and can throw (quota / private mode), so stage the value in an in-memory fallback before setItem and prefer it on read, and never touch localStorage on a per-frame path. Add a matching checklist item. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: K. O. A. <topkoa@gmail.com>
best-practices: client hot-path performance guidance
…tions best-practices: Visualizations (factory pattern + per-instance settings)
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
Fills the biggest remaining hole: §6 previously said the client runtime API was "out of scope for this version of the spec." This replaces that placeholder with a real, ground-truthed §6 — split into a portable normative layer (safe to pin) and a descriptive layer (Host-provided, Host-versioned, explicitly not frozen by v0.1.0). Traced against the actual Host mount path and runtime objects.
This also gives the best-practices performance guidance (#3) a contract to point at — the "SHOULD NOT do per-frame DOM/IO" rule is now normative in §6.4.
New §6 structure
id-derived container, sources its markup fromscreen, and executesscriptonce. No Host-invoked entry point —scriptself-executes and finds its own DOM by the ids it authored. Normative: re-hydration MUST be idempotent (a second execution installs nothing twice); declaringplugin-runtime-idempotent.v1asserts this.capability-pipelines.v1); rawwindowglobals documented as supported-but-legacy and being migrated behind capabilities. No global Host version — per-objectversionsentinels; feature-detect.scriptruns unsandboxed on the ~60fps main thread; SHOULD NOT do per-frame DOM queries / layout reads / IO; MUST NOT subtree-observe a shared container; don't mutate the shell (use contribution registries); suspend loops when hidden; keep state per-instance.Design note
Deliberately kept the API shape descriptive (it's evolving — the Host is migrating globals behind capabilities), while pinning only the stable, portable constraints: idempotence, resource discipline, and "prefer registries/capabilities over shell mutation." This documents reality without over-committing an unstable surface.
Scope & checks
spec/plugin-spec-v1.md§6 +examples/full-plugin/README.mdpointer +CHANGELOG.md[Unreleased]. No version bump.mkdocs build --strictandcheck_versions.pypass; no broken anchors.Merge-order note: touches
spec/plugin-spec-v1.md(§6) andCHANGELOG.md[Unreleased], as does #2 (§8). Different spec sections, but expect a trivial[Unreleased]conflict depending on merge order — easy to resolve.🤖 Generated with Claude Code
Summary by CodeRabbit
destroy()cleanup, settings propagation, and failure-safe behavior.