refactor(css): Phase 2 closed on evidence — FL extraction rejected, orphan cleanup + bundle merge shipped (-2,553 lines)#363
Conversation
…le comment refs
The Oct-2025 Phase-2 spec's extraction destinations were already built and
never wired in: utilities/fl-builder-{components,visibility}.css,
components/layout-{foundation,rows,columns}.css, and
foundations/_fl-responsive-display.scss (a .scss file in a Sass-less
PostCSS pipeline — could never compile). Zero loaders verified
independently (no resources.Get, no @import; only comment mentions).
Gate: converged production build — all css bundles byte-identical,
HTML diffs limited to known noise (sw.js?v= stamp, term-casing races).
Tests: bin/rake test:critical 46/46 (84 screenshots clean) + bin/dtest 46/46.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…,956 lines) The two hash bundles differed by 13 lines (bf72bba adds img.mfp-img padding + fl-photo @860px block). Repointed page/clients.html at the superset and deleted e93d9b85. Compiled output 100% byte-identical: PurgeCSS drops the 2 extra rules from the clients bundle (page has zero mfp-img/fl-photo elements — verified against baseline HTML). Gate: converged production build, empty md5 diff across all bundles; clients/index.html byte-identical. Tests: bin/rake test:critical 46/46 (84 screenshots clean) + bin/dtest 46/46. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gzip evidence, cleanup shipped instead Two read-only audits falsified the Oct-2025 spec: 17 live layout files (not 7); proposed foundations already existed as never-wired orphans; shared-foundation simulation is net-negative on first-visit transfer (+7.1 to +9.8KB gz per page); .fl-visible duplication 41.9% vs 90-95% claimed. Phase 3 must be re-scoped against compiled+gzip before work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR replaces the CSS migration plan, closes Phase 2 tracking items, removes obsolete FL-Builder styles and redundant comments, and updates the clients page to reference a replacement layout bundle. ChangesCSS migration maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… plan supersedes Phase 3 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/projects/2509-css-migration/2026-07-12-css-maintainability-redesign.md`:
- Line 1: Replace every em dash character (—) in the document, including the
heading and the occurrences around lines 45 and 111, with a standard hyphen (-);
leave all other Markdown content unchanged.
In `@docs/projects/2509-css-migration/TASK-TRACKER.md`:
- Around line 623-626: Replace the ambiguous “this commit” reference in the
update log with the actual commit hash or another stable, uniquely identifying
label, while preserving the existing migration details.
- Line 242: Replace every em dash in the changed Markdown lines, including the
Phase Status entry and the referenced lines, with a hyphen while preserving the
surrounding text and formatting.
- Around line 521-523: Update the phase_2_blockers status to reflect that Phase
2 closed on 2026-07-12, matching the established status in the other tracker
entries; leave the Phase 1 and Phase 3 statuses unchanged.
- Around line 255-257: Mark the task lists following the closed work-package
headings WP2.1 and the other referenced closed work packages as superseded/not
applicable, or move those historical plans into an archive section. Ensure no
unchecked items remain presented as active work under the closed headings.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9769f232-4129-406b-b2b1-c9dbfc8c6f2f
📒 Files selected for processing (20)
docs/projects/2509-css-migration/2026-07-12-css-maintainability-redesign.mddocs/projects/2509-css-migration/TASK-TRACKER.mdthemes/beaver/assets/css/2949-layout.cssthemes/beaver/assets/css/3021-layout.cssthemes/beaver/assets/css/3027-layout.cssthemes/beaver/assets/css/3059-layout.cssthemes/beaver/assets/css/3082-layout.cssthemes/beaver/assets/css/3086-layout2.cssthemes/beaver/assets/css/590-layout.cssthemes/beaver/assets/css/701-layout.cssthemes/beaver/assets/css/706-layout.cssthemes/beaver/assets/css/737-layout.cssthemes/beaver/assets/css/components/layout-columns.cssthemes/beaver/assets/css/components/layout-foundation.cssthemes/beaver/assets/css/components/layout-rows.cssthemes/beaver/assets/css/e93d9b85e7803f50c80b8a698f8d12f9-layout-bundle.cssthemes/beaver/assets/css/foundations/_fl-responsive-display.scssthemes/beaver/assets/css/utilities/fl-builder-components.cssthemes/beaver/assets/css/utilities/fl-builder-visibility.cssthemes/beaver/layouts/page/clients.html
💤 Files with no reviewable changes (17)
- themes/beaver/assets/css/components/layout-foundation.css
- themes/beaver/assets/css/utilities/fl-builder-components.css
- themes/beaver/assets/css/components/layout-rows.css
- themes/beaver/assets/css/foundations/_fl-responsive-display.scss
- themes/beaver/assets/css/utilities/fl-builder-visibility.css
- themes/beaver/assets/css/e93d9b85e7803f50c80b8a698f8d12f9-layout-bundle.css
- themes/beaver/assets/css/706-layout.css
- themes/beaver/assets/css/components/layout-columns.css
- themes/beaver/assets/css/701-layout.css
- themes/beaver/assets/css/737-layout.css
- themes/beaver/assets/css/3021-layout.css
- themes/beaver/assets/css/3059-layout.css
- themes/beaver/assets/css/590-layout.css
- themes/beaver/assets/css/3027-layout.css
- themes/beaver/assets/css/3082-layout.css
- themes/beaver/assets/css/2949-layout.css
- themes/beaver/assets/css/3086-layout2.css
| @@ -0,0 +1,117 @@ | |||
| # CSS Maintainability Redesign — approved spec (2026-07-12) | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace em dashes with hyphens.
Line [1], Line [45], and Line [111] use —, which violates the Markdown content guideline. Use - instead.
Also applies to: 45-45, 111-112
🤖 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/projects/2509-css-migration/2026-07-12-css-maintainability-redesign.md`
at line 1, Replace every em dash character (—) in the document, including the
heading and the occurrences around lines 45 and 111, with a standard hyphen (-);
leave all other Markdown content unchanged.
Source: Coding guidelines
| **Duration**: 40-50 hours (4-5 weeks part-time) | ||
| **Target Impact**: 1,900-2,900 lines eliminated | ||
| **Risk Level**: MEDIUM - Systematic extraction from 7 files | ||
| **Phase Status**: ❌ CLOSED 2026-07-12 (sprint 7) — extraction rejected on measurement; orphan cleanup + bundle merge shipped instead |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace em dashes with hyphens.
These changed Markdown lines use —, contrary to the repository guideline. Replace each em dash with -.
Also applies to: 255-255, 283-283, 311-311, 339-339, 369-371, 601-601
🤖 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/projects/2509-css-migration/TASK-TRACKER.md` at line 242, Replace every
em dash in the changed Markdown lines, including the Phase Status entry and the
referenced lines, with a hyphen while preserving the surrounding text and
formatting.
Source: Coding guidelines
| #### WP2.1: FL-Row Foundation Extraction ❌ CLOSED — net-negative on shipped bytes (see Phase header) | ||
| ```yaml | ||
| status: 🔲 Not Started | ||
| status: ❌ Closed 2026-07-12 without implementation (sprint 7 measurement) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Mark the closed work-package tasks as superseded.
These headings now say the work packages closed without implementation, but their following task lists remain unchecked and read like active work. Mark them as not applicable/superseded or move the historical plans into an archive section.
Also applies to: 283-285, 311-313, 339-341
🤖 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/projects/2509-css-migration/TASK-TRACKER.md` around lines 255 - 257,
Mark the task lists following the closed work-package headings WP2.1 and the
other referenced closed work packages as superseded/not applicable, or move
those historical plans into an archive section. Ensure no unchecked items remain
presented as active work under the closed headings.
| phase_1_blockers: NONE - Ready to start immediately | ||
| phase_2_blockers: NONE - Waiting for Phase 1 completion | ||
| phase_3_blockers: NONE - Waiting for Phase 2 completion | ||
| phase_3_blockers: NONE - Phase 2 closed 2026-07-12; Phase 3 WPs must be re-scoped against compiled+gzip output BEFORE any work starts (Phase 1+2 both falsified their specs) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the stale Phase 2 blocker status.
Line [522] still says Phase 2 is “Waiting for Phase 1 completion,” while Lines [242] and [370] state that Phase 2 closed on July 12, 2026. Change this to reflect the closed state.
🤖 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/projects/2509-css-migration/TASK-TRACKER.md` around lines 521 - 523,
Update the phase_2_blockers status to reflect that Phase 2 closed on 2026-07-12,
matching the established status in the other tracker entries; leave the Phase 1
and Phase 3 statuses unchanged.
| - (this commit) — merged `e93d9b85…-layout-bundle.css` (1,956 lines) into its superset | ||
| `bf72bba…` (13-line diff: img.mfp-img padding + fl-photo @860px block) by repointing | ||
| `page/clients.html`. Compiled output 100% byte-identical (PurgeCSS drops the 2 extra | ||
| rules for clients; clients page has zero mfp-img/fl-photo elements). Both suites green. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a stable commit identifier instead of “this commit.”
The update log should record the actual commit hash or a stable label; “this commit” becomes ambiguous after subsequent commits are added.
🤖 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/projects/2509-css-migration/TASK-TRACKER.md` around lines 623 - 626,
Replace the ambiguous “this commit” reference in the update log with the actual
commit hash or another stable, uniquely identifying label, while preserving the
existing migration details.
…content blog post coverage in Phase 0 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… superseded, Phase 0 + rewrite sprints added GOAL OVERVIEW re-goaled (FL burn-down 16→0 replaces the falsified 70-80% line-count target); CUMULATIVE METRICS → burn-down + historical note; Phase 3 WP definitions closed as superseded (preserved in git history + archives); QUICK ACTIONS + navigation repointed to the spec and ownership map; sprint-8 update-log entry added. Spec count fix: 17→16 live FL files (PR #363 merged e93d9b85 away — reviewer finding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Phase 2 of the CSS migration (FL-Builder foundation extraction, Oct-2025 spec) was re-scoped against measured reality before implementation — and rejected on its own metric. What shipped instead is the cleanup the audits actually justified: -2,553 source lines with 100% byte-identical compiled output.
Why the spec was rejected (two independent read-only audits)
css-processor.html; none inline._fl-responsive-display.scssin a Sass-less PostCSS pipeline..fl-visibleduplication is 41.9%, not the claimed 90-95% (453 instances, 263 distinct — breakpoint variants, not copies).fl-node-XXXXrules extraction cannot touch by definition.Same failure mode Phase 1 caught in WP1.2/WP1.4: source-line counting vs compiled+gzip reality.
Commits
5892fe44— delete 6 orphan FL-foundation artifacts + 61 stale comment refs (-597 lines)09cfce21— mergee93d9b85…-layout-bundle.cssinto its 13-line supersetbf72bba…, repointpage/clients.html(-1,956 lines)11a97842— tracker: Phase 2 closed with evidence; Phase 3 flagged for compiled+gzip re-scope before any workGates (per commit)
sw.js?v=stamp, taxonomy term-casing race)mfp-img/fl-photo-*elements; PurgeCSS drops the 2 extra rules —clients/index.htmlbyte-identicalbin/rake test:critical: 46/46, 84 screenshots clean (macOS)bin/dtest: 46/46 (Linux/Docker — CI parity)🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Refactor
Style