Skip to content

feat: support per-lane workflow heights - #255

Open
zhouyuanxinand wants to merge 14 commits into
tt-a1i:mainfrom
zhouyuanxinand:codex/fix-workflow-lane-heights
Open

feat: support per-lane workflow heights#255
zhouyuanxinand wants to merge 14 commits into
tt-a1i:mainfrom
zhouyuanxinand:codex/fix-workflow-lane-heights

Conversation

@zhouyuanxinand

@zhouyuanxinand zhouyuanxinand commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem and value

Closes #250 by allowing a single workflow lane to contain a vertical stage stack without introducing a multi-lane-group model. The result preserves fixed-v1 compatibility while readable-v2 can measure the affected lane independently and keep unrelated lanes compact.

Scope

  • What changed:
    • Added optional lanes[].height with a 104px minimum.
    • Preserved fixed-v1 geometry when the property is omitted or explicitly set to the default; authored v1 heights now fail with actionable diagnostics when a rendered group would not contain a member.
    • Made readable-v2 treat an authored lane height as that lane's independent minimum, including group frames, auto-height, containment, and the rendered SVG.
    • Added height-aware Viewer fitting plus three-stage and five-stage real-browser regressions for the Issue workflow: per-lane height / tall lanes for a vertically-stacked group (one container, N stacked stages) #250 stacked workflow.
    • Merged current main (ed7f4d4, including fix(cli): prevent non-HTML output overwrites without restricting directories #322) in merge commit ab4908b and resolved the binary package conflict by rebuilding archify.zip from the final combined source.
    • Corrected the archive metadata so archify/bin/archify.mjs is the single 100755 entry and the other 76 tracked files remain 100644.
    • Added a package regression that requires the extracted CLI to retain mode 755 and run directly on Unix, in addition to Node-prefixed invocation.
  • What deliberately did not change: release identity, public version, the fixed-v1 baseline without an authored lane height, or the existing multi-lane-group model.
  • No unrelated changes: confirmed.

Stability impact

  • Compatibility and migration risk: existing v1/v2 documents without lanes[].height retain their established behavior. Explicit height: 104 remains geometry-equivalent to omission in fixed-v1. Invalid authored heights fail with structured diagnostics rather than silently clipping group members.
  • Renderer, validator, package, or generated-artifact risk: workflow compiler, Viewer, generated validators, rendered examples, Gallery artifacts, and the canonical ZIP are intentionally updated together.
  • Failure behavior and rollback path: invalid height/group combinations fail validation with a supported repair. Reverting this focused feature and its regenerated artifacts restores the prior lane sizing behavior.

Tests run

Latest exact head ab4908b, Windows development host, Node v22.23.2:

  • node --test test/workflow-compiler.test.mjs test/v1-compatibility.test.mjs test/workflow-compiler-hard-contract.test.mjs test/visual-check.test.mjs test/desktop-reader-browser.test.mjs — all 149 non-browser tests and both Issue workflow: per-lane height / tall lanes for a vertically-stacked group (one container, N stacked stages) #250 browser acceptance tests passed; one production Chrome inspection timed out while four heavy suites were running concurrently.
  • Isolated real-Chrome rerun: node --test test/desktop-reader-browser.test.mjs — 3 passed, 0 failed, 0 skipped. This covers the production reader plus the Issue workflow: per-lane height / tall lanes for a vertically-stacked group (one container, N stacked stages) #250 three-stage and five-stage workflows at all required desktop viewports.
  • node --test test/cli-output-types.test.mjs — 9 passed, 0 failed, 4 symlink-permission skips. All non-symlink fix(cli): prevent non-HTML output overwrites without restricting directories #322 output-type cases passed.
  • Git Bash package smoke: node --test test/cursor-onboarding.test.mjs — 2 passed, 0 failed; the extracted zero-dependency CLI runs directly and through Node.
  • node scripts/generate-brand-marks.mjs --check, node scripts/generate-validators.mjs --check, node ../scripts/check-release-identity.mjs, and node test/golden.mjs — passed.
  • node --test test/gallery.test.mjs — 1 passed, 0 failed.
  • Canonical Node 22 ZIP rebuilt twice from the merged source — exact byte match, 77 entries, exactly one executable entry; SHA-256 CB9483FB06550FCEBDFCC1C2FBA95247400E057C32B6CA1D2D6FE807FC92A76D.
  • Broader Windows/MSYS package run: 22 passed, 5 failed, 2 skipped. The failures are host-only mode/path/symlink limitations (0755 mapping, MSYS conversion of Windows temp paths, and symlink EPERM); the direct extracted-CLI smoke above passed.
  • Broader Windows CLI/output-path run: 57 passed, 11 failed, 7 skipped. Failures are the existing Windows preview-signal and symlink-privilege cases; the newly merged output-type suite passed as reported above.

Prior feature head a82c3e2 was also validated in Ubuntu WSL2 with Node 22: npm test reported 1050 passed, 0 failed, and 4 conditional skips. Its remote CI run was green across Node 18/20/22/24, WebM/browser, ZIP freshness, and Ubuntu/macOS/Windows package smoke. Exact-head remote CI for ab4908b is requested below.

Visual evidence

Generated artifacts

  • Rebuilt after the latest-main merge: archify.zip, with one executable CLI entry and 76 regular-file entries.
  • Existing feature artifacts retained and previously regenerated with the workflow/Viewer changes: archify/examples/*, examples/*, docs/gallery.html, docs/gallery/manifest.json, docs/gallery/artifacts/*, and docs/assets/archify-live-proof.{gif,json}.

Remote CI

  • Previous exact head a82c3e2: all required CI and package-smoke jobs passed.
  • Current exact head ab4908b: GitHub reports MERGEABLE; CI run 34018218223 is action_required and awaits maintainer approval before jobs can start.

Checklist

  • I used a minimal focused change and preserved existing typed JSON behavior unless the issue requires a contract change.
  • I ran the relevant targeted tests and npm test in archify/.
  • I added or updated a regression test for behavioral changes.
  • I checked generated artifacts and package freshness when their sources changed.
  • I removed secrets, private repository content, and customer data from fixtures and screenshots.

ankittecham-arch

This comment was marked as duplicate.

@ankittecham-arch

Copy link
Copy Markdown

Hello

@sunsunsun-java

Copy link
Copy Markdown
Collaborator

Thanks for the PR. The overall direction is good: adding per-lane height is appropriately scoped and is preferable to modeling one logical container as multiple lanes.
However, I found a blocking containment issue in fixed-v1. With the original three-stage reproduction and lane.height: 442, compilation succeeds, but the last node ends at y=494 while the group ends at y=490. The node is therefore still rendered outside the group box. The current validation checks lane containment and logical group membership, but not whether member rectangles are actually contained by the rendered group frame.
There is also a default-equivalence inconsistency: explicitly setting height: 104 changes the fixed-v1 group height from 58px to 62px because the bottom inset depends on whether the property is present. Explicitly specifying the default value should produce the same geometry as omitting it.
Please:

  • preserve the legacy fixed-v1 group inset regardless of whether height is present;
  • validate authored-height v1 group members against the actual rendered group bounds and return an actionable structured diagnostic;
  • add boundary regressions for insufficient height, exact-fit height, negative yOffset, and explicit height: 104;
  • add a v2 test with height: 104 proving that the stacked lane auto-grows while an unrelated lane remains 104px;
  • update from the latest main and rebuild the canonical ZIP from the final source;
  • resolve the reused 2.16.0 release identity, correct the reported test count, run remote CI and the full npm test, and provide the required visual-review evidence.
    Once these items are addressed, the proposed design should satisfy issue workflow: per-lane height / tall lanes for a vertically-stacked group (one container, N stacked stages) #250 without requiring a larger multi-lane-group redesign.

@zhouyuanxinand
zhouyuanxinand force-pushed the codex/fix-workflow-lane-heights branch from c5cae54 to 72ea590 Compare September 2, 2026 09:38
@zhouyuanxinand

Copy link
Copy Markdown
Contributor Author

Rebased this PR onto current main (06dd052) and resolved the binary package conflict by rebuilding archify.zip from the merged source with Node 22.

Updated head: 72ea590.

Validated:

sunsunsun-java
sunsunsun-java previously approved these changes Sep 2, 2026

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for addressing the earlier fixed-v1 group-containment and default-equivalence issues. The code-level boundary tests now look solid, but the feature still fails its intended browser acceptance case.

Using the Issue #250 shape as a readable-v2 workflow — one lane with height: 104, one group, stageA -> stageB -> stageC in the same column with yOffset: 0 / 90 / 180, bottom-to-top edges, and no explicit meta.viewBoxvalidate passes 9/9 and deliver passes with 0 errors / 0 warnings. However, visual-check fails every required desktop viewport due to vertical overflow:

  • 1440x900 -> scrollHeight 1231 (Stage C is below the first screen)
  • 1600x1000 -> scrollHeight 1359
  • 1920x1080 -> scrollHeight 1359
  • 2048x1320 -> scrollHeight 1377

This is the exact product shape the PR is meant to unlock, so artifact validation alone is not sufficient. Please make the tall-lane output fit the required desktop viewports without clipping or an internal scroller, either through height-aware Viewer scaling or an appropriate intrinsic canvas/aspect-ratio solution.

After fixing it, please regenerate the same three-stage artifact and attach:

  1. a corrected 1440x900 light screenshot corresponding to the currently failing view, with all three stages and the full group visible;
  2. the matching dark screenshot and 2048x1320 evidence;
  3. the visual-check --json receipt showing no overflow at 1440x900, 1600x1000, 1920x1080, and 2048x1320;
  4. a browser regression covering this exact Issue #250 case;
  5. a rebuilt canonical archify.zip and green exact-head CI.

Please keep deliver, automated browser containment, and perceptual visual review as separate evidence. Once the corrected artifact and receipt are available, I will re-review the new exact head.

const groupHeaderHeights = groupLaneReserves.map(({ header }) => header);
const groupFooterHeights = groupLaneReserves.map(({ footer }) => footer);
const laneHeights = groupLaneReserves.map(({ header, footer }) => laneH + header + footer);
const laneHeights = groupLaneReserves.map(({ header, footer }, index) => (

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Blocking visual acceptance issue: the new per-lane height can produce a tall portrait viewBox while the Viewer still expands it primarily from the available width. The canonical Issue #250 three-stage stack passes validate and deliver, but visual-check reports vertical overflow at every required desktop viewport; at 1440x900 the page is 1231px tall and Stage C is below the first screen. Please make the resulting artifact height-aware (Viewer fit or intrinsic aspect ratio), add a browser regression for the exact three-stage repro, and attach corrected light/dark screenshots plus a passing four-viewport visual-check receipt.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for addressing the earlier tall-workflow Viewer issue. I re-reviewed exact head 95635b1700199afe9afe4869bf549f9067111105, and the previous desktop-overflow finding is resolved in the tested reproduction.

On macOS with Node 22.19.0 and real Chrome, the following suites passed together: workflow-compiler.test.mjs, v1-compatibility.test.mjs, workflow-compiler-hard-contract.test.mjs, visual-check.test.mjs, and desktop-reader-browser.test.mjs — 152 passed, 0 failed, 0 skipped. This includes both the three-stage and five-stage browser regressions.

I also independently delivered the three-stage Issue #250 fixture and ran visual-check --json. At 1440x900, 1600x1000, 1920x1080, and 2048x1320, the page has no horizontal or vertical overflow, no internal diagram scroll range, and no clipped nodes or group frames. The receipt passes with no diagnostics. I inspected the 1440x900 light and dark screenshots: all three stages and the complete group are visible. There is still substantial blank space above the stack; this is a composition observation, not a renewed overflow blocker or a claim of complete perceptual acceptance.

One packaging defect still blocks merging:

[P1] Preserve the CLI executable bit in archify.zip. The committed archive records archify/bin/archify.mjs as 100644, while rebuilding the same head with Node 22.19.0 produces 100755. All 77 archived file contents match; the differing entry metadata is this executable mode. Consequently, cmp against the canonical rebuild fails, and the extracted entry point lacks the Unix executable bit. Please regenerate the archive with the tracked executable mode preserved and verify direct execution of the extracted CLI, in addition to Node-prefixed invocations.

Please sync with current main, resolve the binary archify.zip conflict by rebuilding from the final sources, verify byte-for-byte package freshness, and rerun the relevant checks and full suite. CI on this head is currently action_required, with no passing remote checks. Update the PR description with the current scope and validation evidence, including the Viewer and visual-check changes.

I did not run the full local npm test suite in this re-review. The earlier overflow objection is superseded by the successful browser evidence above; the remaining change request is for the package defect and outstanding integration/CI gates.

@zhouyuanxinand

Copy link
Copy Markdown
Contributor Author

Addressed the latest P1 at 572c3b7.

  • Merged current main (d8e4daf) and resolved the binary conflict by rebuilding archify.zip from the final sources in WSL2.
  • The rebuilt 77-entry archive is byte-for-byte fresh; its archify/bin/archify.mjs entry records Unix mode 100755. After extraction, the CLI is mode 755 and both direct execution and node-prefixed doctor succeed.
  • Added a Unix package regression for the extracted executable mode and direct CLI invocation.
  • Final WSL2 npm test: 1054 tests, 1050 passed, 0 failed, 4 conditionally skipped; this includes the Issue workflow: per-lane height / tall lanes for a vertically-stacked group (one container, N stacked stages) #250 browser and visual-check regressions.

The PR description now follows the repository template and separates automated/browser evidence from perceptual review. GitHub Actions was triggered for this head but is action_required before any jobs are created, so maintainer approval is still needed to run remote CI. Please re-review after that gate is available.

@zhouyuanxinand

Copy link
Copy Markdown
Contributor Author

Resolved the approved-run failures at exact head a82c3e2.

Root cause: the previous WSL2 packaging preparation preserved archify/bin/archify.mjs as executable, but also accidentally recorded the other 76 archive entries as 100755. That made both zip-freshness and the Node 22 archive reproducibility test fail; Node 24 was cancelled by matrix fail-fast rather than failing independently.

The replacement archive was built from a real WSL2 Git clone of head 572c3b7, so it now contains exactly one 100755 entry (the CLI) and 76 100644 entries. Its SHA-256 is 090047ac9ef5b45b78f7695e152c2a85477010b0431ffe0bd3c5165d3d5da4a7.

WSL2 validation for the corrected artifact:

  • targeted package gates: 21 tests, 20 passed, 0 failed, 1 conditional skip;
  • full npm test with real Chromium: 1054 tests, 1050 passed, 0 failed, 4 conditional skips;
  • exact rebuild comparison: passed;
  • extracted CLI: direct and Node-prefixed doctor both passed.

The new exact-head workflow run is currently action_required before jobs can start. Please approve that run and re-review this head.

@zhouyuanxinand

Copy link
Copy Markdown
Contributor Author

Synced this branch with current main (ed7f4d4) in exact head ab4908b.

The only merge conflict was the binary archify.zip; I rebuilt it from the final combined source rather than choosing either side. Two canonical Node 22 builds byte-match (CB9483FB06550FCEBDFCC1C2FBA95247400E057C32B6CA1D2D6FE807FC92A76D), with 77 entries and only archify/bin/archify.mjs marked 100755.

Exact-head validation includes:

GitHub now reports the PR as mergeable with no content conflicts. Please approve/run the fork CI for ab4908b and re-review this exact head.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I compared current main ed7f4d4 with this PR at ab4908b, using the public CLI, real Chrome, and the delivered screenshots. The results narrow the product need, so I would like to pause merging and agree on scope before asking for another implementation round.

First, the earlier containment and Viewer fixes are acknowledged. The archive-mode defect was also verified fixed at a82c3e2. This comment is a design/scope discussion, not a renewed report of those resolved defects or an approval of all integration checks on the latest head.

What the comparison showed:

  • The single-container, three-stage vertical workflow is already possible on main: use schema v2, omit the fixed meta.viewBox, and center the authored yOffset values at -90 / 0 / 90. This preserves the group, stage order, spacing, and vertical connections. deliver passed 9/9 checks, and the delivered artifact passed browser checks at 1440x900, 1600x1000, 1920x1080, and 2048x1320. I also inspected the screenshot. The original v1 JSON still fails unchanged on both main and this PR.
  • The clear new benefit is independent sizing in a mixed-lane workflow. In our three-lane reproduction, main produced lane heights [276, 270, 270] and a 768x1032 canvas. Setting the stacked lane's height to 104 in this PR produced [276, 104, 104] and a 768x700 canvas, reducing canvas height by about 32%. Main overflowed the tested desktop viewports; the PR artifact passed all four. This is a useful, demonstrated improvement.

My proposed direction is to focus on independent automatic lane sizing in v2, rather than adding an authored-height API to solve the original single-lane example:

  1. Document the existing v2 solution for the original example, including the fact that yOffset is relative to the lane's content center.
  2. Retain the independent-measurement work, but evaluate a v2 layout change without introducing lane.height in this iteration. Currently height: 104 acts as both a minimum and an opt-in to a different sizing policy, and can also enable Viewer fitting. That is a broader public contract than a height value alone suggests.
  3. Establish the compatibility boundary before changing the default v2 behavior. Independent sizing can change existing geometry; explicit offsets, routes, fixed viewBoxes, and checked-in examples need before/after evidence. We should not silently reinterpret authored constraints to obtain a smaller canvas.
  4. Base Viewer fitting on the measured artifact and readability constraints, rather than the presence of an authored height field. Keep this work only as broad as the demonstrated layout need requires.
  5. Leave fixed-v1 unchanged for this feature. The general visual-check improvements for scrolling, clipping, and theme verification are useful independently and can be proposed separately.

If a real use case requires authors to specify a minimum lane height, we can evaluate that API separately with a single, explicit meaning.

Please start with feedback on this scope and the compatibility tradeoffs; there is no need to do another broad rewrite before we agree on the direction. We should have made this product-scope decision earlier in the review process. Passing tests and resolving the implementation findings are valuable, but we also need to agree on the public behavior we are committing to maintain before merging.

Refine readable-v2 independent lane measurement without adding public lane.height, preserve fixed-v1 and authored-geometry compatibility, and fit the Viewer from compiled geometry.
@zhouyuanxinand

Copy link
Copy Markdown
Contributor Author

Addressed the latest review in d9c2000 and merged current main (1072200). GitHub now reports the PR as conflict-free / mergeable.

What changed after the review:

  • Removed the proposed public lanes[].height field. It remains rejected by the schema.
  • Left fixed-v1 behavior unchanged, including the original stacked-source failure and the exact 700×400 compatibility geometry.
  • Kept independent lane measurement only for readable-v2 workflows that have an implicit canvas and a real same-lane/same-column vertical stack (distinct yOffset values). Absolute authored pins (via, labelAt, channelX, channelY) and explicit meta.viewBox retain the existing shared-height v2 geometry. Route presets remain compiler-owned and can use independent measurement.
  • Documented the existing v2 representation with centered offsets (-90 / 0 / 90) and clarified that yOffset is relative to the lane content center.
  • Made Viewer fitting depend on compiled/measured SVG geometry (data-reader-fit="intrinsic-height" when a measured lane exceeds the 104px baseline), not on the presence of an authored sizing field. The Viewer preserves canonical SVG geometry and enforces a 6px projected node-text floor.
  • Removed the unrelated general visual-check changes from this PR.

Compatibility evidence now locked by tests:

  • Current-main shared layout: lane heights [276, 270, 270], canvas 768×1032.
  • This branch's independently measured layout: [276, 104, 104], canvas 768×700 (about 32% less height).
  • Explicit viewBox and absolute-route-pin fixtures remain [276, 270, 270].
  • The checked-in non-stacked v2 example preserves its exact SVG SHA-256; fixed-v1 has byte-for-byte regression coverage.

Validation:

  • WSL2 full suite exercised 1,096 tests: all behavior tests passed; its sole initial failure identified that archify.zip had been built with the Windows rather than canonical Linux toolchain. I rebuilt it with scripts/build-zip.sh under WSL2, and the exact cross-time-zone archive gate now passes (1/1).
  • Workflow compiler suite: 42/42 passed.
  • Checkout artifact + README proof reproducibility: 27/27 passed under WSL2.
  • Real Chrome desktop Viewer regressions: 4/4 passed, including the 3-stage and 5-stage stacked workflows.
  • Extracted final ZIP package smoke: passed on Linux.
  • Final archify.zip SHA-256: 690b6df7dc90235025a089d8bb9157dec6c0aa1b1ecaade93b33faae8ff109c8.

Ready for re-review.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: e6263125-5a34-4add-8af4-422e3d497ac1

📥 Commits

Reviewing files that changed from the base of the PR and between 1072200 and d9c2000.

⛔ Files ignored due to path filters (1)
  • archify.zip is excluded by !**/*.zip
📒 Files selected for processing (34)
  • archify/SKILL.md
  • archify/assets/template.html
  • archify/examples/dataflow-product-analytics.html
  • archify/examples/lifecycle-agent-run.html
  • archify/examples/sequence-cache-miss-request.html
  • archify/examples/web-app-rendered.html
  • archify/examples/workflow-agent-tool-call-rendered.html
  • archify/renderers/workflow/README.md
  • archify/renderers/workflow/workflow-compiler.mjs
  • archify/test/adaptive-reader-layout.test.mjs
  • archify/test/desktop-reader-browser.test.mjs
  • archify/test/workflow-compiler.test.mjs
  • docs/assets/archify-live-proof.json
  • docs/gallery.html
  • docs/gallery/artifacts/agent-run.lifecycle.html
  • docs/gallery/artifacts/agent-tool-call.workflow.html
  • docs/gallery/artifacts/async-job-roundtrip.sequence.html
  • docs/gallery/artifacts/cache-miss.sequence.html
  • docs/gallery/artifacts/deployment-release.lifecycle.html
  • docs/gallery/artifacts/event-stream.dataflow.html
  • docs/gallery/artifacts/incident-response.workflow.html
  • docs/gallery/artifacts/product-analytics.dataflow.html
  • docs/gallery/artifacts/production-deployment.architecture.html
  • docs/gallery/artifacts/release-delivery.workflow.html
  • docs/gallery/artifacts/web-app.architecture.html
  • docs/gallery/manifest.json
  • examples/checkout-platform-delta.html
  • examples/checkout-platform-delta.receipt.json
  • examples/dataflow-product-analytics.html
  • examples/lifecycle-agent-run.html
  • examples/sequence-cache-miss-request.html
  • examples/web-app-rendered.html
  • examples/web-app.html
  • examples/workflow-agent-tool-call-rendered.html

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Summary

Summary

Adds optional per-lane workflow heights with a 104px minimum. Readable-v2 now measures stacked nodes independently, sizes lane and group frames from authored or measured heights, validates containment, and reports structured diagnostics when members exceed rendered bounds.

Fixed-v1 preserves existing geometry when lanes[].height is omitted or set to 104px. Existing documents, explicit viewBoxes, absolute pins, route presets, and the release identity remain compatible.

Viewer fitting now supports measured intrinsic-height workflows. Generated examples, gallery artifacts, receipts, hashes, and the canonical ZIP were refreshed. Archive permissions now mark only archify/bin/archify.mjs as executable.

Validation

Author-reported validation includes targeted and full test suites, real-Chrome viewport regressions, artifact freshness checks, executable archive extraction tests, and reproducible ZIP builds.

The provided evidence does not include independently observed checks at the current head. Static test changes do not establish browser or perceptual acceptance. The latest GitHub Actions run was awaiting maintainer approval.

Walkthrough

Readable-v2 workflows now measure vertically stacked lanes independently. The compiler marks oversized implicit stacks as intrinsic-height diagrams. Adaptive reader shells use measured text size to select desktop widths. Tests, examples, documentation, and artifact receipts were regenerated.

Changes

Intrinsic-height workflow layout

Layer / File(s) Summary
Independent lane measurement and compiler output
archify/renderers/workflow/*, archify/test/*, archify/SKILL.md
Readable-v2 measures stacked lanes independently, preserves shared geometry for authored viewBoxes or absolute pins, emits the intrinsic-height marker, and adds compiler and browser regression coverage.
Adaptive reader sizing
archify/assets/template.html, archify/examples/*
Reader shells accept marked intrinsic-height diagrams and derive minimum width from SVG text readability, while preserving existing non-wide and special-mode behavior.
Generated artifact reader updates
docs/gallery/artifacts/*
Regenerated gallery artifacts include intrinsic-height detection, readable-scale calculation, and measured minimum-width selection.
Regenerated receipts and manifests
docs/gallery.html, docs/gallery/manifest.json, docs/assets/archify-live-proof.json, examples/checkout-platform-delta.receipt.json
Artifact hashes and byte counts were updated for regenerated outputs.

Priority: ➖ Normal — Schedule the per-lane height change because it broadly affects readable-v2 workflow layout, group containment, viewer fitting, and generated artifacts while addressing the medium-severity stacked-stage issue.

Estimated code review effort: 4 (Complex) | ~60 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d9c20

The intrinsic-height workflow path is covered through compilation, reader sizing, generated artifacts, and regressions, with no remaining merge-blocking issue identified.

🚥 Pre-merge checks | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Contribution Scope ⚠️ Warning The PR description is complete in structure: it explains issue #250, the intended behavior, compatibility, failure handling, rollback, tests, visual evidence, and generated artifacts. However, the fin… Align the PR description with the final implementation: describe inferred per-lane measurement, omit claims about the public lanes[].height contract and authored-height diagnostics, and update compatibility and failure behavior accordingl…
Validation Evidence ❓ Inconclusive Required validation evidence is incomplete, not a reproduced code defect. The PR adds appropriate compiler, Viewer, fixed-v1, containment, and Issue #250 browser regression coverage. PR-head artifact … A maintainer must inspect the workflow and proposed code, approve the pending fork CI run, and rerun/complete all required final-head jobs for ab4908b, including the Node test matrix, real-browser job, ZIP freshness, and package smoke. Pr…
Full details: Contribution Scope

Explanation

The PR description is complete in structure: it explains issue #250, the intended behavior, compatibility, failure handling, rollback, tests, visual evidence, and generated artifacts. However, the final candidate does not implement the contract that the description states. In the checked-out final head, workflow.schema.json rejects lanes[].height through additionalProperties: false, and the compiler test explicitly asserts that lane.height remains outside the public schema. The compiler instead infers independent lane height only for an implicit v2 vertical stack with no meta.viewBox or absolute route pins. This contradicts the description's claims about an optional lanes[].height, a 104px minimum, authored-height validation, and authored heights controlling readable-v2 sizing. The linked issue supports the inferred auto-growth alternative, so this is a scope-description mismatch, not a confirmed runtime defect.

Resolution

Align the PR description with the final implementation: describe inferred per-lane measurement, omit claims about the public lanes[].height contract and authored-height diagnostics, and update compatibility and failure behavior accordingly. Alternatively, restore the described authored-height schema and implementation. Re-run and record checks for the actual final head, not only the ab4908b head named in the description.

Full details: Validation Evidence

Explanation

Required validation evidence is incomplete, not a reproduced code defect. The PR adds appropriate compiler, Viewer, fixed-v1, containment, and Issue #250 browser regression coverage. PR-head artifact checks are consistent: the Gallery manifest matches all 11 artifact bytes and SHA-256 values; the embedded Gallery manifest matches the manifest file; live-proof GIF and scene digests match; archify.zip has 77 entries, exactly one executable entry (archify/bin/archify.mjs at 755), and its packaged contents and modes match the tracked PR-head inputs. However, GitHub Actions run 34018218223 for exact head ab4908b is completed with conclusion action_required and has no jobs, so required remote CI did not run. The reported full npm test result is for older head a82c3e2; the final-head report lists targeted suites but not current-head npm test, which CONTRIBUTING.md requires for runtime, Skill, generated-content, and shared-test changes. The Visual evidence section also does not give an explicit perceptual-review status or link the original revision for the claimed reused review. Older-head CI success cannot establish final-head acceptance.

Resolution

A maintainer must inspect the workflow and proposed code, approve the pending fork CI run, and rerun/complete all required final-head jobs for ab4908b, including the Node test matrix, real-browser job, ZIP freshness, and package smoke. Provide a reproducible npm test result from archify/ at the final head (or an equivalent exact-head CI result). Record the perceptual visual review separately from automated/browser checks; either perform it under the stated comparable conditions or link the original review revision and state its explicit status and why the intervening changes do not invalidate it.

Warning

Some tools did not complete. Review the errors below.

🔧 ast-grep (0.45.2)
archify/examples/dataflow-product-analytics.html

ast-grep timed out on this file

docs/gallery/artifacts/deployment-release.lifecycle.html

ast-grep timed out on this file

archify/examples/lifecycle-agent-run.html

ast-grep timed out on this file

  • 20 others
🔧 SkillSpector (2.9.5)

SkillSpector batch scan produced no output


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.

❤️ Share

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

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.

workflow: per-lane height / tall lanes for a vertically-stacked group (one container, N stacked stages)

4 participants