Skip to content

chore(devex): keep formatters off generated files - #107510

Closed
webjunkie wants to merge 4 commits into
chore/devex-codegen-projectionsfrom
chore/devex-projections-formatter-exempt
Closed

webjunkie wants to merge 4 commits into
chore/devex-codegen-projectionsfrom
chore/devex-projections-formatter-exempt

Conversation

@webjunkie

Copy link
Copy Markdown
Contributor

Problem

  • A projection renderer had to emit exactly the bytes oxfmt or Biome would leave alone, or the next format run turned generated output into a diff.
  • The model catalog carried formatter emulation for that: a second Biome-shaped render and a line-width guard. Every new renderer would have to copy it.

Changes

  • A *.generated.ts file belongs byte for byte to its generator. .oxfmtrc.json and products/desktop/biome.jsonc skip the glob instead of listing files one by one. A renderer only returns valid TypeScript.
  • The model catalog renders one text for web and desktop. The Biome style and check_fits are gone. The desktop copy changes shape only, not content.
  • The MCP ui-apps registry keeps its generator's raw output, because its oxfmt pass now skips the file. The ci-mcp drift check regenerates the same bytes.
  • Running the real formatters inside the runner was the alternative. It was dropped because regenerating the model catalog would then need the desktop workspace's node_modules, and ci-python would need two formatter toolchains.

How did you test this code?

  • hogli build:projections --check passes. Regenerating the ui-apps registry twice gives the same bytes.
  • biome ci passes on the three desktop outputs, and tsc --noEmit passes for the desktop shared package.
  • Ran the model catalog, runner and invariant tests locally.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

Automatic notifications

  • Publish to changelog?

Docs update

The "Side generators" section in the type system guide says to name TypeScript outputs *.generated.ts.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

  • Stacked on chore(devex): stop new side generators and gate projections together #107222.
  • Codex (gpt-6-astra) suggested a real formatter stage in the runner. This PR takes the formatter-exempt route instead, for the Node and CI cost named under Changes.
  • CodeRabbit CLI (--deep) ran over both layers together: no finding in this layer.
  • Skills: /writing-pr-descriptions, /shipping-pr-programs, /stacking-prs, /reviewing-with-coderabbit.

https://claude.ai/code/session_01JBHJiD5i8S2xTqE38wVVga

…lain

Projection renderers had to emit exactly what oxfmt or Biome would leave
alone, or a later format run turned generated output into a diff. The
model catalog carried formatter emulation for that (two styles and a
line-width guard), and every new renderer would have to copy it.

A *.generated.ts file now belongs byte for byte to its generator:
.oxfmtrc.json and the desktop biome.jsonc skip the glob instead of
listing files one by one. A renderer only has to return valid
TypeScript, and regenerating stays pure Python with no Node in the loop.

Running the real formatters inside the runner was considered and
dropped: regenerating the model catalog would then need the desktop
workspace's node_modules, and ci-python would need two formatter
toolchains.

- The model catalog renders one text for web and desktop; the Biome
  style and check_fits are gone. The desktop copy changes shape only.
- The MCP ui-apps registry keeps its generator's raw output, since its
  oxfmt pass now skips the file. ci-mcp regenerates the same bytes.

Claude-Session: https://claude.ai/code/session_01JBHJiD5i8S2xTqE38wVVga
…ctions-formatter-exempt

# Conflicts:
#	.oxfmtrc.json
Master added an entry that oxfmt had formatted. With generated files
exempt from oxfmt, the registry holds the raw generator output, so the
ci-mcp drift check expects the unformatted form.

Claude-Session: https://claude.ai/code/session_01JBHJiD5i8S2xTqE38wVVga
@posthog

posthog Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 1 should fix, 0 consider.

Published 1 finding (view the review).

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 441b835.

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

🚨 Trunk lane — universal lane

This PR is assigned to the universal lane. It cannot merge in parallel with other PRs, so it can take longer to merge. Ask dev-ex if you think this is wrong.

✅ Complexity (TypeScript) — clean

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

⚠️ Duplication (TypeScript) — 1 new duplicated block (worst 1803 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/desktop/packages/shared/src/model-catalog.generated.ts:10 products/tasks/frontend/modelCatalog.generated.ts:10 506 1803
✅ Bundle size — no change

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 68.88 MiB · no change

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.57 MiB · 22 files no change █████████░ 85.2% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login)
src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
3.58 MiB · 628 files no change █████████░ 88.8% of 4.03 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
7.37 MiB · 2,326 files no change █████████░ 88.4% of 8.34 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/index.tsx
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
854 B src/scenes/ChunkLoadErrorBoundary.tsx
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
Size File
301.8 KiB ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
267.6 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.4 KiB src/lib/api.ts
85.5 KiB src/products.tsx
69.1 KiB src/lib/lemon-ui/icons/icons.tsx
63.9 KiB src/lib/utils/eventUsageLogic.ts
38.7 KiB ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js
33.9 KiB ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js
28.3 KiB src/scenes/scenes.ts
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
301.8 KiB ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs
271.7 KiB src/taxonomy/core-filter-definitions-by-group.json
267.6 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
153.7 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
100.4 KiB src/lib/api.ts
98.5 KiB ../packages/quill/packages/quill/dist/index.js
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js
85.5 KiB src/products.tsx

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

✅ Toolbar bundle — eager 2.37 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.37 MiB · 19 files no change ████░░░░░░ 41.4% of 5.72 MiB
Deferred (lazy) 2.10 MiB · 44 files no change n/a — loads on demand
Loader dist/toolbar.js 1.2 KiB no change █░░░░░░░░░ 6.0% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
791.8 KiB dist/toolbar/toolbar-app-TDLXUZLE.css
650.8 KiB dist/toolbar/chunk-chunk-OLZXKW3U.js
483.6 KiB dist/toolbar/chunk-chunk-LP5DDLVQ.js
138.3 KiB dist/toolbar/chunk-chunk-FVYKO6VU.js
131.8 KiB dist/toolbar/chunk-chunk-FDH2IBXT.js
75.2 KiB dist/toolbar/toolbar-app-K2QOGJR4.js
69.0 KiB dist/toolbar/chunk-chunk-TSAL54PB.js
35.6 KiB dist/toolbar/chunk-chunk-GL4SRUHV.js
21.0 KiB dist/toolbar/chunk-chunk-Z4YQYAC3.js
6.8 KiB dist/toolbar/chunk-chunk-DV7IWQNF.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

✅ Dist folder size — no change

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 944.91 MiB · no change

ℹ️ MCP UI apps size — 33 app(s), 17630.1 KB JS

Built size of each MCP UI app (main.js + styles.css).

App JS CSS
debug 597.9 KB 196.2 KB
action 454.1 KB 196.2 KB
action-list 564.2 KB 196.2 KB
cohort 453.1 KB 196.2 KB
cohort-list 563.2 KB 196.2 KB
email-template 452.9 KB 196.2 KB
error-details 469.1 KB 196.2 KB
error-issue 453.8 KB 196.2 KB
error-issue-list 564.1 KB 196.2 KB
experiment 561.3 KB 196.2 KB
experiment-list 564.9 KB 196.2 KB
experiment-results 566.3 KB 196.2 KB
feature-flag 566.8 KB 196.2 KB
feature-flag-list 570.5 KB 196.2 KB
feature-flag-testing 457.3 KB 196.2 KB
inline-scan 453.6 KB 196.2 KB
insight-actors 562.3 KB 196.2 KB
invite-email-preview 452.3 KB 196.2 KB
llm-costs 559.3 KB 196.2 KB
session-recording 455.3 KB 196.2 KB
survey 454.7 KB 196.2 KB
survey-global-stats 561.9 KB 196.2 KB
survey-list 564.9 KB 196.2 KB
survey-stats 561.9 KB 196.2 KB
trace-span 453.5 KB 196.2 KB
trace-span-list 564.1 KB 196.2 KB
vision-observation-list 563.3 KB 196.2 KB
workflow 453.4 KB 196.2 KB
workflow-list 563.5 KB 196.2 KB
loops-review 457.8 KB 196.2 KB
query-results 774.1 KB 196.2 KB
render-ui 857.0 KB 196.2 KB
visual-review-snapshots 457.9 KB 196.2 KB
✅ Playwright — all passed

All tests passed.

View test results →

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 772e5cbe-2133-4752-bf7e-74c9702a1f3e

📥 Commits

Reviewing files that changed from the base of the PR and between b3b057b and 441b835.

⛔ Files ignored due to path filters (2)
  • products/desktop/packages/shared/src/model-catalog.generated.ts is excluded by !**/*.generated.*
  • services/mcp/src/resources/ui-apps.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (6)
  • .oxfmtrc.json
  • docs/published/handbook/engineering/type-system.md
  • posthog/object_tags/projection.py
  • products/desktop/biome.jsonc
  • products/tasks/scripts/model_catalog_projection.py
  • tools/hogli-commands/hogli_commands/projections.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

Formatter configurations now skip all *.generated.ts files. Documentation and comments describe this convention. The model catalog projection now uses one style and emits the same rendered text for web and desktop outputs. It no longer performs formatter-specific rendering or line-width validation.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 441b8

The formatter-policy and catalog-output changes show no actionable issue in the inspected contracts, so the PR is mergeable with normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 441b8

The change affects generation and formatting rather than application access or privileges. The inspected renderer still uses the checked-in catalog, and no new untrusted input path was identified. Some build and deployment coverage remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected outputs are generated catalog data for web and desktop consumers. The inspected changes do not add runtime authority, a tenant boundary, or a new external input to their generator; deployment-wide exposure was not established.

Trust Boundaries and Controls

  • observed — The generator continues to serialize checked-in catalog values through Style.s, while the runner requires the declared output keys. The removed line-width check enforced presentation, not either of these controls.

Resilience and Maintainability Implications

  • inferred — Byte-based drift detection supports recovery from interrupted sequential writes, but recovery depends on a later check and regeneration; no transactional filesystem guarantee was found.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description includes all required sections and clearly explains the problem, changes, testing, release status, documentation update, and agent context. It is self-contained and sufficiently specif…
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@webjunkie
webjunkie marked this pull request as ready for review September 28, 2026 07:06
@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team September 28, 2026 07:07
@pr-assigner-resolver-posthog

Copy link
Copy Markdown

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

  • @PostHog/team-devex (owners.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). The locator after each owner is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

@trunk-io

trunk-io Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@webjunkie
webjunkie requested review from a team and removed request for a team September 28, 2026 07:12
@posthog

posthog Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review

PostHog Review

Found 1 should fix.

Comment on lines +126 to +128
// A *.generated.ts file belongs byte for byte to the generator that wrote it;
// its drift check compares those bytes, so no formatter may touch it.
"includes": ["**/*.generated.ts"],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FLASH MODE - Faster, but stupid, use regular ReviewHog for a heavy review

Disable Biome assists for generated files

should_fix bug

Issue description

This override disables only the formatter, but the desktop config still enables the organizeImports assist. If a generated file has imports that are not in Biome’s preferred order, biome ci can report a mismatch and biome check --write can reorder them. That breaks the byte-for-byte generator contract for *.generated.ts files.

Why we think it's a valid issue
  • Checked: products/desktop/biome.jsonc:61-67 enables organizeImports; products/desktop/biome.jsonc:125-131 disables only the formatter for **/*.generated.ts.
  • Found: Ran Biome 2.2.4 ci on a temporary *.generated.ts file with unsorted imports. It reported assist/source/organizeImports and exited with an error. biome check --write reordered those imports.
  • Impact: Generated files with unsorted imports can fail biome ci and be changed by biome check --write, breaking the generator-owned byte-for-byte contract.
Suggested fix

Disable Biome assists in this override, or turn off organizeImports for these files, so Biome does not rewrite generated output.

Prompt to fix with AI (copy-paste)
## Context
@products/desktop/biome.jsonc#L126-128

<issue_description>
This override disables only the formatter, but the desktop config still enables the `organizeImports` assist. If a generated file has imports that are not in Biome’s preferred order, `biome ci` can report a mismatch and `biome check --write` can reorder them. That breaks the byte-for-byte generator contract for `*.generated.ts` files.
</issue_description>

<issue_validation>
- **Checked:** `products/desktop/biome.jsonc:61-67` enables `organizeImports`; `products/desktop/biome.jsonc:125-131` disables only the formatter for `**/*.generated.ts`.
- **Found:** Ran Biome 2.2.4 `ci` on a temporary `*.generated.ts` file with unsorted imports. It reported `assist/source/organizeImports` and exited with an error. `biome check --write` reordered those imports.
- **Impact:** Generated files with unsorted imports can fail `biome ci` and be changed by `biome check --write`, breaking the generator-owned byte-for-byte contract.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Disable Biome assists in this override, or turn off `organizeImports` for these files, so Biome does not rewrite generated output.
</potential_solution>

@webjunkie

Copy link
Copy Markdown
Contributor Author

Closing: generated files keep the repo formatting, the same as the OpenAPI flow's output. Exempting them would make the side lanes diverge from the main flow.

@webjunkie webjunkie closed this Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant