Skip to content

feat(codegen): infer workspace token modules - #311

Merged
aram-devdocs merged 1 commit into
mainfrom
codex/init-from-workspace-tokens
Jun 21, 2026
Merged

feat(codegen): infer workspace token modules#311
aram-devdocs merged 1 commit into
mainfrom
codex/init-from-workspace-tokens

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

  • discover conventional workspace package token modules from plumb init --from apps/<name>
  • parse exported literal TS/JS token objects conservatively without evaluating JavaScript
  • infer spacing, radius, colors, type sizes, families, and weights into generated plumb.toml

Validation

  • cargo fmt --all -- --check
  • cargo test -p plumb-codegen
  • cargo test -p plumb-cli --test init_from
  • cargo clippy -p plumb-codegen -p plumb-cli --all-targets --all-features -- -D warnings
  • RUSTDOCFLAGS='-Dwarnings' cargo doc --workspace --no-deps
  • just validate
  • PLUMB_CHROME="/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" scripts/noise-scoreboard.sh --live

Scoreboard

  • noise-kitchen-sink: 24 total
  • example.com: 0 total
  • ui.shadcn.com: 299 total
  • news.ycombinator.com: 410 total

Review Notes

  • The token parser intentionally accepts only plain decimal numeric literals and string literals. Unsupported numeric forms such as 1e3, 1_000, 0x10, 0b10, 0o10, 100n, and bare 12px are skipped rather than partially parsed.

@aram-devdocs
aram-devdocs merged commit ab126c9 into main Jun 21, 2026
20 of 25 checks passed
@aram-devdocs
aram-devdocs deleted the codex/init-from-workspace-tokens branch June 21, 2026 01:19
@github-actions

Copy link
Copy Markdown
Contributor

I now have everything needed. Let me write the structured review.


PR #311feat(codegen): infer workspace token modules

Crates touched: plumb-codegen (primary), plumb-cli/tests


1. Determinism

Clean. No SystemTime/Instant, no HashMap/HashSet in observable output (token maps go through IndexMap via Config), no env reads in library code.

Sort stability:

  • read_sorted_paths sorts ReadDir output before iteration → deterministic walk order. ✓
  • exports.sort_by in merge_literal_token_module uses a two-key comparator (token_sort_key, name) → stable. ✓
  • leaves.sort_by uses (token_path_key, path) → stable. ✓
  • walked.ts_token_modules.sort() + .dedup() in walk::walk. ✓
  • summary.sort() in infer_config operates on (u8, String, String). ✓

One oddity worth noting:

ts_tokens.rs:405-412 — redundant inner sort+dedup. merge_literal_token_module calls sort_unstable/dedup on spacing.scale, radius.scale, type_scale.scale, and type_scale.weights before returning. infer_config (lib.rs:201-204) then runs sort_and_dedup on the same four fields immediately after merge_token_modules returns. The inner pass is effectively a no-op at that point because the outer pass follows immediately. Not wrong — but noisy. A follow-up cleanup can remove the inner pass; the outer one in infer_config is the canonical dedup site.


2. Workspace Layering

Clean. plumb-codegen sits between plumb-config (layer 4) and plumb-cli (layer 6), depending only on plumb-core and plumb-config. No upward or diagonal imports.

The new serde_json = { workspace = true } dep is already in the workspace graph (used by at least one other crate). Adding it to plumb-codegen is fine; package_json_declares_workspaces in walk.rs:1474-1482 is the only caller.

ts_tokens.rs carries #![forbid(unsafe_code)] implicitly from the workspace [lints]. No unsafe block anywhere in the diff. ✓


3. Error Handling

Clean. All Result-returning functions propagate via ?. thiserror-backed CodegenError is used consistently. Library code uses unwrap_or_default() only where the default value is immediately validated (name.is_empty() check at walk.rs:1327).

#[allow(clippy::unwrap_used, clippy::expect_used)] appears on the ts_tokens.rs test module (ts_tokens.rs:1103) but is absent from the new tests added to walk.rs:1493-1537. If the workspace denies clippy::unwrap_used, the walk.rs tests would fail on a future strict clippy pass. The existing pre-PR walk tests use the same pattern, so this is not a regression — but the inconsistency should be normalized.


4. Test Coverage

Good breadth:

  • ts_tokens.rs unit tests cover the happy path and unsupported numeric literal edge cases.
  • walk.rs unit tests cover workspace root discovery and package.json field requirements.
  • init_from.rs integration test exercises the full end-to-end path from plumb init --from <app-subdir> through to the written plumb.toml.

One gap:

init_from.rs:97-106contains assertions instead of insta snapshot. The test validates structured TOML output with eight fragile substring checks. A structural regression (e.g., key renamed, section reordered) would pass all eight checks yet degrade the output. The testing rules specify insta golden snapshots "for anything structured." The existing init_from.rs tests follow the same contains pattern (pre-existing), so this PR didn't introduce the anti-pattern, but the new test for the highest-value feature (workspace inference) is the right place to raise the bar. Recommend snapshotting written with insta::assert_snapshot!.


5. Documentation

TokenModuleImport and all its fields are doc-commented. merge_literal_token_module is doc-commented. TokenSourceKind::TokenModule variant is doc-commented. # Errors is not needed on merge_literal_token_module (returns TokenModuleImport, not Result). Private/crate-internal functions are exempt. ✓

The CHANGELOG omission is intentional — release-please generates entries from conventional commits, and feat(codegen): will produce one on the next release. ✓


Punch List

# File Lines Issue Class
1 crates/plumb-codegen/src/ts_tokens.rs 405-412 Inner sort_unstable/dedup is redundant — infer_config runs sort_and_dedup on the same fields immediately after; remove the inner pass Warning
2 crates/plumb-cli/tests/init_from.rs 97-106 Structured TOML output validated with contains checks; should be insta::assert_snapshot! per testing rule Warning
3 crates/plumb-codegen/src/walk.rs 1493-1537 Test module uses .unwrap() without #[allow(clippy::unwrap_used)], inconsistent with ts_tokens.rs:1103 Warning
4 crates/plumb-codegen/src/ts_tokens.rs 1009-1012 has_joined joins all hints then does substring search — "discolor" matches "color", "monospace" wouldn't but edge cases exist; document the intentional liberal-match design Warning

No blockers found. Determinism invariants, workspace layering, error propagation, and public-API documentation are all correct.

Verdict: APPROVE

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