Merged
Conversation
The UI ships every rule inside its JS (webpack style-loader) and injects them as separate <style> tags at runtime — 92 live on a loaded desk, ~200 over a longer session. This page therefore links no application CSS at all. That is expensive at render time, not just at load: Chrome consults one RuleSet per stylesheet for every element it restyles, so a ~5,800-element recalculation performed well over a million rule-set consultations. Measured on production 2026-09-11 at 12-30us per element, against well under 1us for a normal document, and 33.4s of style recalculation in a single session. The UI side now extracts one merged `styles.<hash>.css` and lists it in the same manifest.json these bundles already come from, so this only has to read one more key and emit one more <link>. OPTIONAL BY DESIGN, so this is safe to deploy on its own and should go FIRST: a UI build that predates the change has no `styles.css` key, `data.styles` is null, the guard in index.tpl emits nothing, and the runtime <style> injection keeps working exactly as it does today. The non-manifest branch never sets the key at all, which the template also tolerates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Links the app stylesheet emitted by ui-team's perf/css-single-stylesheet build. Optional by design: with a UI build that has no styles.css in its manifest this changes nothing, so it is safe ahead of the UI deploy — and must land first, since the reverse order renders the app unstyled.
Duy, 2026-09-11: the Billing page's promo banner takes about a second to appear, and the tabs feel sluggish. Measured on the live test endpoint: `payment.catalog` takes **1919 ms** while every other payment call on the same page takes 162-184 ms. The cause is an `await` inside a for-loop — one Stripe round trip per priced row, end to end, 6 of them at roughly 310 ms each. It matters more than a slow read usually would. The Billing page cannot decide whether a promotion is real until the catalog lands (the banner, the struck prices and the campaign modal all gate on the catalog's actual discount, by design, so the page can never advertise a cut Stripe is not giving). So that 1.9 s was the promo's time-to-appear. Fanning the lookups out takes it to roughly the cost of one call. Each task keeps its OWN try/catch, so a dead price id still leaves that row without an amount and the FE still falls back to its offline figure — Promise.all can never reject here, which is what stops one bad id emptying the catalog. Verified equivalent, not assumed: stubbed Stripe at the measured 310 ms and compared the two implementations across all-prices-good, one-dead-id and every-id-dead. Identical result shape and identical call count in all three, 5.0x faster. The five tests CI runs all pass. Next step if this is ever not enough: cache the catalog server-side. Prices change a few times a year and every page load currently re-reads them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#217) hub.set_privilege and hub.delete_contributor only pushed to the member being changed. Every other admin with the permission matrix open kept the old role, or the removed member, until they reopened the panel. Both now end with one hub.members_changed broadcast to the hub's online sockets ({ hub_id, change: "privilege" | "removed", users }), sent after the per-member writes so a refetch reads committed rows. The existing per-member pushes are unchanged. notifyMembersChanged sits beside notifyMemberJoined and, like it, never throws. Co-authored-by: Drumee Dev <drumee@debian.local.drumee> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
![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: 620099139d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+2809
to
+2812
| await notifyMembersChanged(this, this.hub.get(Attr.id), { | ||
| change: "privilege", | ||
| users, | ||
| }); |
There was a problem hiding this comment.
When callers use the ACL-exposed hub.set_member_privilege endpoint—for example, to update one member with an expiry—the adjacent implementation writes permission_grant and returns the refreshed list without invoking notifyMembersChanged; only set_privilege reaches this new call. Those updates therefore still leave every other open permission matrix stale, so the single-member privilege writer should emit the same broadcast.
Useful? React with 👍 / 👎.
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
3 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.
Whole-branch promotion
test→preview, requested by Natrix (2026-09-13).origin/previewwas back-merged intotestfirst (6200991). See Back-merge note — it needed one deliberate resolution.What ships
fix(hub): role changes and removals reach every open permission matrix (fix(hub): role changes and removals reach every open permission matrix #217, Huân)perf(payment): fetch the catalog's Stripe prices in parallelfeat(page)stylesheet link (see below)7 files, +316/−22.
#217 is additive and safe to deploy ahead of the UI
It adds a new
hub.members_changedpush (notifyMembersChanged) sent after the per-member writes; it never throws, and no existing payload or service name changes. A UI that does not know the event simply ignores it.testwithout a PR), so the live-sync behaviour will not be visible on UAT until that is resolved. Nothing breaks in the meantime.Back-merge note — one deliberate resolution, please sanity-check
The back-merge was not content-neutral on its own: it pulled
client/page.js(+14) andclient/templates/index.tpl(+3) — thefeat(page): link the app stylesheetcode — back ontotest, becausepreviewstill carries it.test, Revert: link the app stylesheet #215 genuinely reverted that feature (ad1eb85, a clean revert ofd90bfe6, −17).preview, revert: optmization test #216abb87d9is an EMPTY commit — titledrevert: optmization test, it changes no files, and its parentb698923is what added the feature. So the preview-side revert never took effect.I resolved to
test's state (feature removed), which is what promotingtestmeans and what the real revert intended; this promotion therefore completes the removal onpreview.This is inert either way, verified rather than assumed:
MiniCssExtractPlugin.loaderis commented out inwebpack/module.js:11on both ui branches, so nostyles.cssis emitted, and the live UAT page today links zerostyles.css—data.stylesresolves to null and the template guard emits nothing. Removing the code changes no rendered output.Final merge is content-neutral against
origin/test;node --checkpasses onclient/page.js.Verification
f9eca98f(2026-09-13 11:53Z).../schemas/...from the local checkout (mine sits onmain); the asserted pattern is present on bothpreviewandtestofdrumee/schemas. Not a real failure.drumee/schemastestandprevieware already content-identical, so that repo has nothing to promote this round.🤖 Generated with Claude Code