Repository navigation
fix: docs-kit conforms to conformance-kit v0.4.0 (snapshot lock, auto-finalize, Ractor canary, strict audit) - #103
Conversation
Adds the caller, bumps conformance-kit to ~> 0.3 and lets Dependabot keep both pins current. The conformance check is not required yet: it turns green, and becomes required, with #91. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
A tag bump without the gem (or the reverse) fails the conformance version check, so both now arrive in a single multi-ecosystem pull request. Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Dismissed because Cubic found issues in a newer review.
DocsKit::Snapshot.for kept its cache in an unguarded class ivar
(`@cache ||= {}`): threads that missed a cold key together each read and
parsed the manifest and raced to store it (32 threads parsed it up to 26
times). The cache now lives in DocsKit::Store, a copy of conformance-kit's
per-Ractor store (the kit is a development dependency only), and its
check-then-set runs under that Ractor's lock. A Snapshot builds its
registries in initialize, so a cached instance carries no lazy memo.
Refs #91
Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
Every DocsUI::Code render resolved its Rouge theme with Object.const_get, every nav render constantized each page's view class, and the brand logo, SEO, landing and optional-mcp memos were filled by whichever request got there first. DocsKit.finalize! resolves the themes and every registered page's view class once into frozen (Ractor-shareable) maps and fills those memos; the engine runs it in after_initialize, after the host's to_prepare configure, unless code reloading is on. DocsKit.configure re-opens it, and a theme assigned directly afterwards still wins. Configuration#openapi_document kept its mtime memo in unguarded ivars: threads that missed together each parsed the spec. The memo now lives in a per-Ractor DocsKit::Store cache whose check-then-set runs under a lock. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
…nd fibers Puma serves docs-kit from threads and Falcon from fibers in one reactor. conformance-kit's "thread-safe under contention" and "fiber-safe under an Async reactor" examples now run DocsKit::Scope.with (each request keeps its own version and leaves none behind) and an archived-version sidebar (DocsKit::Snapshot.for + Configuration#nav_groups: one snapshot, one sidebar), each call yielding mid-request so the others run inside it. The thread example fails on the old unguarded snapshot cache. async joins the test group so the fiber example runs rather than skips. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
…ctor conformance-kit's "runs inside a non-main Ractor" canary (Ruby 4.0; skipped with a reason below) now builds a SearchIndex from literal pages, assembles llms.txt and llms-full.txt from a Configuration built inside the Ractor, and builds a literal registry's NavItem tree. Two blockers were docs-kit's own: Configuration::DEFAULT_NAV was an unshareable Proc (every Configuration.new read it) and a registry's page list was an unfrozen Array in a class ivar. The page DSL now makes every value shareable on write, DEFAULT_NAV is a shareable lambda, and DocsUI::Callout::LEVELS is frozen all the way down. Rendering cannot run in a Ractor yet: the Phlex, commonmarker and Nokogiri canaries are pending with the upstream reason (yippee-fun/phlex#1055, commonmarker's Proc-defined options, nokogiri's C extension), and turn red the day one of them passes. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
conformance-audit (Ruby 4.0) found constants a non-main Ractor could not read: ApiClient::DEFAULTS held Method objects (never shareable; now capture-free lambdas), DocsUI::Code::FORMATTER and Engine::JAVASCRIPT_PATH were unfrozen, and the install generator's MIGRATIONS held an unshareable block, PACKAGE_FLOORS unfrozen hashes and MIN_RUBY a Gem::Version. Gem::Version.new hands out process-wide cached instances, so MIN_RUBY and Migration#to keep a String and build the Gem::Version where they compare. MigrationRegistry.default is built at load (DEFAULT) instead of an unguarded `@default ||=`. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
.rubocop.yml inherits conformance-kit's config/rubocop.yml (the rubocop-thread_safety cops, now in the bundle at >= 0.8) and every offence is fixed rather than baselined: - DocsKit.configuration is built at load instead of a lazy `||=`, so no first request can race to build a second one; the remaining reset is boot/spec-only and says so inline. - DocsUI::Page's class macros (title, eyebrow, description, on_page) make each value shareable on write; they run in the class body at load. - The docs_kit:og task's one-shot screenshot server thread, the store's main-only fallback and the release-kit spec's block-form Dir.chdir are disabled inline with the reason; the snapshot contention spec, which starts threads on purpose, is excluded from NewThread with its reason. - The scoping spec's stand-in host records around_actions in a closure, not a class ivar. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
.conformance.yml sets what `conformance-kit check` (and the conformance workflow on Ruby 4.0) holds docs-kit to: audit.require boots a minimal Rails app and eager-loads the gem's zeitwerk tree, the engine's controllers, the generators and the og task's generator; namespace DocsKit + DocsUI with strict_autoloads (0 unloaded autoloads); the `rspec --tag ractor` canary; probes for docs-kit. The strict audit has no unallowlisted finding. main_only carries only state that truly is main-only, each with its reason: Rails::Engine's, ActionController's and Thor's own class state, the copied store's below-3.4 fallback, and DocsKit.@configuration, which holds host per-request callables by public API. SIMPLECOV_MINIMUM lets the canary stage (ractor.env) lift the suite's 80% floor, which a handful of examples cannot meet. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
The conformance workflow runs the Ractor canary and the strict audit on Ruby 4.0, so the whole suite and RuboCop run there as well, next to 3.3 and 3.4. AGENTS.md lists the lane and `conformance-kit check`. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
…t values Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
The caller's tag and the Gemfile pin move together (the conformance job's version handshake checks one against the other). No tracked lockfile pins the kit: the root Gemfile.lock is gitignored, and docs/ doesn't bundle it. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
conformance-kit 0.4's ancestor: rules replace the 25 name: entries for the state Rails::Engine, ActionController::Base and Rails::Generators::Base (Thor) keep on docs-kit's engine, controllers and generators. Each rule is narrowed to those frameworks' own ivars, so state docs-kit kept on the same classes would still be audited. The audit allowlists the same 28 names as before, with no stale entries; docs-kit's own state (DocsKit.@configuration, the Store fallback) keeps its name: entries. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
The engine skipped DocsKit.finalize! while code reloading was on, so a development app resolved theme and page view classes per render. It now finalizes after boot in every environment. With reloading on it also adds a to_prepare after boot, which runs after the host's to_prepare configure (that re-opens the configuration) on every reload, so the resolved view classes are the reloaded ones. The host's per-request callables are left as they are: they close over app state, so the configuration stays main-Ractor-only, and finalizing never tries to make it shareable. A child-process boot spec covers both environments, a reload, and callables that close over mutable app state. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
README's Configure section says when the engine finalizes (after boot in every environment, again after a development reload), what is frozen, and that the host's callables stay as given, so the configuration is main-Ractor-only and a Ractor passes its own. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
The [Unreleased] entries move under a 1.4.0 heading (no date: the release sets it), with an Upgrading from 1.3 note, the development finalize and the conformance-kit 0.4 move. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
|
Running ultrareview automatically — this PR appears to change concurrency-sensitive code such as locks, queues, or retries, where a missed race may only surface under production load, so a deeper multi-pass review is worth running. I'll post findings when complete. |
There was a problem hiding this comment.
Ultrareview ran automatically — this PR appears to change concurrency-sensitive code such as locks, queues, or retries, where a missed race may only surface under production load, so a deeper multi-pass review is worth running.
3 issues found across 41 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="spec/docs_ui/page_spec.rb">
<violation number="1" location="spec/docs_ui/page_spec.rb:182">
P3: The assertion passes vacuously when a macro stops storing its value: title/eyebrow/description fall back to nil and on_page to the config default, and nil and symbols are always Ractor-shareable. Also assert each getter equals the value written, so "shareable" cannot mask "nothing stored".</violation>
</file>
<file name="spec/docs_kit/finalize_spec.rb">
<violation number="1" location="spec/docs_kit/finalize_spec.rb:47">
P3: This `allow(Object).to receive(:const_get)` stub intercepts every Object.const_get in the process during the example, not just the theme lookups the test is about. Any incidental const_get (a framework autoload, a future change to the read path) turns the example red, and the `not_to have_received(:const_get)` assertion can't distinguish it. Scope the stub and assertion to the two theme names.</violation>
</file>
<file name="spec/docs_kit/shareable_constants_spec.rb">
<violation number="1" location="spec/docs_kit/shareable_constants_spec.rb:18">
P2: `Rouge::Formatters::HTML.new.freeze` has the same shallow-freeze problem, so this assertion fails too: freezing the formatter instance leaves its internal ivars (notably the default unfrozen `@opts = {}` Hash its `initialize` stores) mutable, and `Ractor.shareable?` recurses into them and returns false. Build it shareable instead: `FORMATTER = DocsKit::Shareable.make(Rouge::Formatters::HTML.new)` in app/components/docs_ui/code.rb.</violation>
</file>
…e old one Registry#finalize! caches each page's view class by view name. Setting view_namespace afterwards left that cache in place, so a page kept rendering the class from the old namespace. The setter now clears it, and the entries resolve under the new namespace. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
Configuration#openapi_document cached each Document in a per-Ractor map keyed by configuration, which kept every configuration that had read a spec (and its parsed Document) alive for the Ractor's lifetime: one per reset_configuration! in a host's suite. The memo is back on the configuration, so a replaced one takes its Document with it. The check-then-set still runs under a per-Ractor lock, so threads that miss together parse once and share one Document. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
Below 3.4 a non-main Ractor keeps Store values in Ractor-local storage, which has no lock. The initializer ran inside the ||=, so threads that missed together each stored and returned their own value, and a cache's callers could hold different maps and mutexes. The value is now built first, and the check-then-store runs no Ruby code between the read and the write: every thread gets the first value stored. The branch had no spec. It is now covered on every Ruby by stubbing both capability checks, with a Thread.pass in the initializer to force the race. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
The formatter was only shallow-frozen. Rouge's HTML formatter keeps no ivars today, so that was already shareable, but a Rouge in the allowed range that does keep state would leave the constant unreadable from a non-main Ractor. A deep freeze holds for every version. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
- finalize_spec checks that the two theme names are not looked up again, not that no Object.const_get happens at all. - shareable_spec skips its Ractor examples on an engine without Ractor, so the plain-freeze fallback example can run there. - page_spec checks that each macro stored the value it was given before calling it shareable (nil and symbols are always shareable). - An empty SIMPLECOV_MINIMUM now means the default floor instead of an ArgumentError at load. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
A page whose view class didn't exist at boot is still looked up when it is read, so it turns up once written. The README promised no lookup at all. Refs #91 Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
Without labels, Dependabot uses its ecosystem labels (ruby, github_actions), which aren't in the taxonomy, so its PRs would carry no type or area label. The labels go on the multi-ecosystem group. Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
…tored (#17) * fix(store): racing threads on a non-main Ractor get the first value stored Below Ruby 3.4 a non-main Ractor's Store keeps values in Ractor-local storage, where no lock can be shared. `||= [yield]` read the slot, ran the initializer, then wrote, so threads that raced on the build each kept their own value (a forced-race spec saw 8 for 8 threads). The value is now built before the check-and-publish: every thread returns whichever was stored first. Found by zoolutions/docs-kit#103. Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU * docs(store): state the remaining publish-step race Claude-Session: https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU
Summary
docs-kit now passes conformance-kit v0.4.0 end to end. This PR started as the caller rollout:
.github/workflows/conformance.yml(now at@v0.4.0),conformance-kit ~> 0.4and.github/dependabot.yml. It also carries #91, soconformance / Conformance (Ruby 4.0)goes green on its merits.Tier 1, thread-safe. RuboCop inherits
conformance-kit: config/rubocop.yml, andrubocop-thread_safety >= 0.8is in the bundle. Every offence was fixed; none was baselined.DocsKit::Snapshot.for, the bug the audit found. The cache was an unguarded@cache ||= {}. When 32 threads hit a cold version together, they parsedmanifest.jsonup to 26 times. The cache now lives inDocsKit::Store, which is per Ractor, and the check-then-set runs under a lock, so a cold version is parsed once. A Snapshot also builds its registries ininitialize, so a cached instance has no lazy memo left to race on.Configuration#openapi_document: the mtime memo goes through the same per-Ractor store, under a lock. Threads that missed together used to get up to 16 distinct Documents. Now they get one.DocsKit.configurationis built at load instead of with||=.MigrationRegistry.defaultis a constant built at load.DocsUI::Page's class macros and the registry DSL make each value shareable when it is written. They only run in class bodies, at load.Tier 2, fiber-safe. The new
spec/docs_kit/contention_spec.rbrunsDocsKit::Scope.withand an archived-version sidebar (Snapshot.forplusnav_groups) through"thread-safe under contention"and"fiber-safe under an Async reactor". Every call yields in the middle (Thread.pass/Async::Task#yield).asyncis now in the test group.Tier 3, Ractor-clean.
DocsKit.finalize!runs from the engine'safter_initializein every environment, with no app code, after the host'sto_prepareconfigure.to_prepareafter boot. That hook runs after the host'sto_prepareconfigure on every reload. The configure re-opens the configuration, then the hook re-finalizes it against the reloaded classes.SearchIndex,llms.txt/llms-full.txtand a literal registry'sNavItemtree inside a non-main Ractor, on 4.0.conformance-audit --strict(strict_autoloads) reports 0 findings. To get there:DEFAULT_NAV,ApiClient::DEFAULTS(it held Method objects),Callout::LEVELS,Code::FORMATTER,Engine::JAVASCRIPT_PATH,MIGRATIONSandPACKAGE_FLOORSare now shareable.MIN_RUBYandMigration#tokeep a String, becauseGem::Version.newreturns process-wide cached instances.DocsKit::Storeis a copy ofConformanceKit::Store, andDocsKit::Shareable.makeis a capability-gatedRactor.make_shareable.name:entries became 3name:entries plus 3ancestor:rules.Rails::Engine,ActionController::BaseandRails::Generators::Base. Each rule is narrowed withivars:to that framework's own ivars.name:entries:DocsKit.@configurationand the two Store fallback ivars.Test plan
These runs used CI-shaped bundles: the published daisyui, no sibling path gem, one lockfile per Ruby, with conformance-kit 0.4.0 resolved.
bundle exec conformance-kit checkon Ruby 4.0.6 exits 0:conformance-audit --strictthree times: on 0.3.0 with the old config, on 0.4.0 with the old config, and on 0.4.0 with the ancestor rules. Each run allowlists 28 names. The sorted name lists are identical, and no run has a stale section.bundle exec rakeon Ruby 3.3.8, 3.4.8 and 4.0.6, after the review fixes: 1174 examples, 0 failures each.finalize_specfailed 9 of 9.DEFAULT_NAVand on the registry's@pages.engine_specfailed 2 of 4: "finalizes it with code reloading on" and "again after every code reload".boot_specfailed 2 of 4: the development boot and the reload both reportedfinalized: false.view_namespaceafter finalize returned the old namespace's class.spec/docs_kit/boot_spec.rbboots a real minimal Rails app, configured the way the README and the generator do it (DocsKit.configureinto_prepare). Itsbrand,nav,version_badgeandimportmaplambdas close over a mutable app object.Rails.application.reloader.reload!.docs/, Ruby 3.4.11):bundle exec rspec(request and Playwright system specs): 15 examples, 0 failures. RuboCop is clean.RAILS_ENV=development bin/rails runner:finalized?istrueat boot andtrueafterreloader.reload!. After the reload, the cached page view is the reloaded class (equal?the current constant, not the pre-reload one), and the registry is the reloadedDoc.conformance / Conformance settings.conformance / Conformance (Ruby 4.0)loggedconformance-kit 0.4.0 satisfies the workflow's v0.4.conformance-kit check passed.Remaining from #91
conformance / Conformance settingsandconformance / Conformance (Ruby 4.0). That is a repo-settings change, left for the maintainer. It is also why this PR saysRefs, notCloses.pending, each with its upstream reason, and each turns red the day it starts passing:Phlex::ATTRIBUTE_CACHE, Give each Ractor its own attribute cache so components render on non-main Ractors yippee-fun/phlex#1055.DocsUI::Markdown: commonmarker defines its options with unshareable Procs.DocsKit::MarkdownExport: nokogiri's C extension raisesRactor::UnsafeError.Deviations & judgment calls
DocsKit::Storeinstead ofConformanceKit::Store. The issue's Decision names the kit's store, but the kit is a development dependency only. I copied it (MIT, same author). Its two below-3.4 fallback ivars are allowlisted with the kit's own reasons.DocsKit.@configurationis allowlistedmain_only. By public API it holds host-supplied per-request callables (brand,nav,version_badge,importmap), so it can never be shareable. Deep-freezing it at boot would also break two things: theto_preparereconfigure on a dev reload (c.seo.description = ...mutates the SeoConfig in place), and any host spec that callsDocsKit.configure. Per the issue's Decision,finalize!covers the derived readers. Code inside a Ractor builds its ownConfigurationand passes it in, which is what the canary does. The maintainer may want to weigh in on this one.lib/generators/**. That surfaced three docs-kit constants, now fixed. The Thor/Rails generator class state it brings in is covered by theRails::Generators::Baserule.configurestill re-opens a finalized config and its registries.DocsKit.configurationexists from load.brand_logonow raises at boot, in every environment, not on first render.spec/conformance_eager_load.rb. It boots a minimal Rails app, becauseDocsUI::PagereadsRails.application.routesat load and thedocs/app has its own bundle. It then eager-loads only the gem's own zeitwerk loader, noteager_load_all.ractor.envsetsSIMPLECOV_MINIMUM(default 80) to 0, because a--tag ractorrun can't reach 80%.Page's macros (disable-next: written once, in the class body, at load).DocsKit.reset_configuration!(boot and specs only).Thread.new(a CLI; the thread is killed inensure).Dir.chdir.NewThreadexclude.spec/docs_kit/snapshot_contention_spec.rbstarts threads on purpose. It is excluded fromThreadSafety/NewThread, with the reason in the config.required_ruby_version >= 3.3), so I left the floor alone. The new 4.0 lane is added toci.yml's rake matrix.choreis replaced by the issue'sbugandcore.devopsstays.components,generators,openapianddxcome frombin/labels infer.Round 2 (minor release, auto-finalize, conformance-kit 0.4):
after_initialize, the latest boot hook the gem already used, which now finalizes in development too. Inside it, the engine registersapp.reloader.to_prepare { DocsKit.finalize! }when reloading is on.config.to_prepareblock to the reloader, so on each reload the host's configure runs first and the re-finalize second.config.to_preparein the engine body would have run before the host's block instead.finalize!stays a documented call (README), rather than running at the end ofDocsKit.configure.DocsUI::Pageneeds Rails routes.configureruns on every dev reload and in every host spec, so finalizing there would only add a second pass inside Rails.c.seo.description = ...mutates in place). It would also buy no Ractor access, because the object stays main-only for its callables. What finalize resolves is frozen, as are the DSL and macro values.brand_logoexamples are plain strings or need a real file, and afile:mark still re-reads on mtime change after the boot memo.Gemfile.lockis gitignored, anddocs/Gemfile/docs/Gemfile.lockdon't bundle conformance-kit. So no lockfile changed, and.github/dependabot.ymlkeepsdirectory: /.[Unreleased]entries now sit under## [1.4.0], with an empty[Unreleased]above it as in the 1.3.0 cut. The heading has no date, which the release sets. It adds an "Upgrading from 1.3" note (nothing to change for ato_prepareinitializer; three things now fail loudly).Review of abd71a8 (13 cubic findings, all resolved):
view_namespaceafter finalize now drops the cached view classes.DocsKit::Store's Ractor-local fallback (below 3.4, off the main Ractor): racing threads now all get the first value stored. That branch is now covered on every Ruby. conformance-kit's ownConformanceKit::Storehas the same||= [yield], so the kit's copy needs the same fix (not done here; another repo).Code::FORMATTERis built withDocsKit::Shareable.make. It was already shareable with this Rouge, but the deep freeze holds for any Rouge in the gemspec's range.const_getassertion is scoped to the two theme names.shareable_specskips its Ractor examples on an engine without Ractor.page_specchecks the stored values, not just their shareability.SIMPLECOV_MINIMUMmeans the default floor.labels: ["dependencies", "devops"].job.workflow_ref.~> 0.4: the handshake only rejects an older gem.Refs #91
https://claude.ai/code/session_01SXwrW19u1qWNW3fm2pUnrU