Automattic / Automattic/blocks-engine

Extend ConversionFindingContract across the PHP/TypeScript boundary and record the engine scope split

Open
#1,328 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
PHP
Stars
14
Forks
2
Avg merge
2h 10m
Merged PRs (30d)
561

Description

Related to #242, workstream 2.

This repo ships a PHP transformer and a TypeScript package that, read from their READMEs, appear to do the same job. They do not. This issue records the boundary, fixes the one real defect it hides, and stops the question being re-derived.

> **Note:** this issue was rewritten shortly after opening. The original text offered "converge the two engines" as one of three plausible readings. Investigation since then rules that out on evidence, and the ask below is narrower: contract parity plus a scope statement, explicitly **not** unification. The corrected reasoning is included so the rejected option stays traceable.

## The two implementations

| | PHP | TypeScript |
|---|---|---|
| Path | `php-transformer/src/` | `packages/blocks-engine/src/` |
| Package | `automattic/blocks-engine-php-transformer` | `@automattic/blocks-engine` v0.2.2 |
| Source LOC | 67,801 | 14,813 |
| Commits, last 90 days | 1,132 | 23 |
| Consumers | `static-site-importer` (pinned `0.7.0`) | `data-liberation-agent`, and Studio via its bundled copy (`^0.2.2`) |

## They are not the same transform

| | PHP transformer | TypeScript engine |
|---|---|---|
| Input | HTML document + CSS | section spec (structured, already extracted) |
| Goal | **visual fidelity** | **native, editable blocks** |
| Verification | 8 `VisualParity` probe/comparator classes, `tools/visual-parity`, static cascade comparison | golden-output capture only — no visual-parity gate exists |

Three pieces of evidence, in increasing order of weight:

1. **Consumption is narrower than the README implies.** All 48 `data-liberation-agent` import sites resolve to the `/theme` subpath — 70 distinct symbols, overwhelmingly theme assembly (fonts, segmentation, region audit, variation hoisting, sanitize). There is **zero** direct use of the root `convert()` / `convertReport()` export. The root converter is reached only transitively, from `theme/chrome-parts.ts` and `theme/chrome-parts-builders.ts`, to convert header/footer chrome.

2. **The input contracts differ.** The consumer's own architecture documentation names the engine's core as *"section spec → native blocks"* — a pure transform over structured specs already captured from a site. The PHP transformer's input is a raw HTML document plus its CSS. These are not the same starting point.

3. **The downstream consumer already made this call internally, and wrote down the principle.** It keeps a local emit-blocks path rather than routing it through the engine core, because that path *"preserves the owned source DOM instead of re-emitting markup from captured specs — a genuinely different transform."* The stated rule: *"share the core where the output is identical; keep strategies separate where they truly differ."*

Unifying the engines would require implementing spec→native-blocks in PHP **and** a visual-parity gate in TypeScript — building both missing halves in order to merge two things that were separated for a defensible reason.

### Runtime is not the constraint

Worth recording because it is the assumption most likely to be raised, and it is wrong. `static-site-importer` already drives the PHP transformer from Node via `spawnSync` in `tools/run-fixture-matrix.mjs`. Its php-wasm tooling is for zstd-decoding `.fig` archives, not for hosting the transformer.

So PHP *can* reach the Node consumer's runtime. That does not help, because the transform that consumer needs is not the one the PHP transformer implements. The boundary is behavioural, not environmental.

## The real defect: the finding contract stops at the language boundary

Workstream 2 formalized `Contract/ConversionFindingContract.php`:

```
SCHEMA = 'blocks-engine/php-transformer/conversion-finding/v1'
CLASSIFICATION_FIELDS = ['reason_code', 'repair_bucket', 'pattern_family']
```

The TypeScript side references that schema **0 times**, and carries a parallel vocabulary in `theme/fallback-diagnostic.ts`:

```ts
export type FallbackReasonCode = 'dropped_images' | 'text_coverage_below_floor' | 'decorative_asset_triaged';
export type FallbackRepairClass = 'recover_dropped_media' | 'restructure_section_blocks' | 'replace_with_structural_block';
```

Different field names, different value sets, no shared assertion helper, no version handshake.

This is the same failure mode #1371 fixed between this repo and `static-site-importer` — one producer, one consumer, two vocabularies drifting silently — except it sits inside this repo and was not covered when that work landed. Downstream, `data-liberation-agent` and Studio consume diagnostics that nothing validates.

**Two engines emitting different diagnostics is expected and fine. Two engines describing the same *kind* of conversion loss in mutually unintelligible terms is not.** A reader aggregating findings from both cannot tell whether `dropped_images` and some PHP-side reason code mean the same thing.

## Proposed work

1. **Extend `ConversionFindingContract` v1 across the language boundary.** The schema is already versioned and explicit. Port `assertFinding()` and `classify()` to TypeScript, with a shared fixture proving both sides classify the same input identically. Contract parity does not require, and should not imply, merging the engines.

2. **State the scope of each engine where readers meet it.** The TypeScript README currently opens with *"Convert HTML → blocks … Build a whole block theme"* — an accurate description of the PHP transformer's job, which is why the packages read as duplicates. Naming the section-spec input and the native-blocks objective removes the ambiguity.

3. **Record the boundary as an ADR** under `docs/adr/`, including the rejected unification option and the reasoning above, so the decision survives the next person who notices two engines.

Item 1 is the one with a live failure mode. Items 2 and 3 are cheap and prevent recurrence.

## Explicitly out of scope

Consolidating the engines, porting either to the other's language, or deprecating the TypeScript package. The 49:1 commit ratio reflects two products at different maturities with different consumers — not neglect of a duplicate.

---

*AI assistance disclosure: this issue was researched and drafted by Claude Sonnet 4.6 running in Claude Code, operated by @chubes4. The AI read `blocks-engine` at `trunk` `24b115ac`, measured per-area commit counts and source LOC, enumerated the 70 consumer-imported symbols and their subpaths, traced the root-converter reachability, compared the verification gates on both sides, checked how Node already invokes the PHP transformer, and verified the zero-reference contract gap quoted above. It also rewrote this issue after its own initial framing was contradicted by that evidence. No code was changed. Reviewed by a human before posting.*

Contributor guide

Open the contributing guide

Research direction

Start with Contract/ConversionFindingContract.php and theme/fallback-diagnostic.ts to compare the existing finding fields and vocabularies. Trace the TypeScript package README and the /theme entry points, then inspect the existing contract work from #1371. Done means a shared fixture demonstrates matching classification, the README states the section-spec scope, and an ADR under docs/adr/ records the boundary and rejected unification.

Written by the indexing model from the issue text.

Assessment

Tech stack
php, typescript
Domain
backend, documentation, testing
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.