Repository navigation
Conversation
EDH-330 Passepartout: Expand planner to no-upscale output decisions
Sourcehttps://linear.app/edhorsagents/document/prd-zero-sense-multi-experiment-bench-edf4de601568 Branch: Commit: What to buildExtend the Passepartout experiment from basic orientation and content-box selection into a fuller output planner that accounts for no-upscale behavior and max-canvas constraints. The slice should still avoid media processing and should remain demoable through the Zero check gate plus documented planner cases. Acceptance criteria
Verification
Stack / PR statusStacked above EDH-328 in the Passepartout stack. Implementation is complete locally, but Graphite PR submission is blocked by EDH-335 because this checkout has no Git remote configured and Graphite cannot infer the GitHub owner/name. |
|
Warning Review limit reached
Your plan currently allows 4 reviews/hour. Refill in 8 minutes and 14 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds integer aspect-ratio fitting helpers Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Linear Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcff6fa700
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
e4ea580 to
fab5a58
Compare
dcff6fa to
403157c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@experiments/passepartout-layout-planner/src/main.0`:
- Around line 66-79: The plan_output function currently calls content_box with
the raw border_px which can make border_px*2 exceed the selected export spec max
dimensions and produce invalid content_w/content_h; update plan_output (after
calling select_export_spec) to validate and clamp/reject border_px so that
border_px*2 <= spec.max_w and border_px*2 <= spec.max_h (e.g., set border_px =
min(border_px, spec.max_w/2, spec.max_h/2) or return an error), then call
content_box with the validated border_px and compute content_w/content_h from
that safe box; ensure references to OutputDecision, select_export_spec,
content_box, content_w, content_h and border_px are preserved.
In `@tests/passepartout-planner-fixtures.test.mjs`:
- Around line 44-85: The test currently reimplements fixture logic in
fixturePasses and asserts its own JS computation instead of running the actual
planner fixtures; remove or stop using fixturePasses and instead invoke the
planner/fixtures runtime (e.g., call the existing fixture script or execute the
planner with target fixtures via zeroJson/graph execution) to get real runtime
outputs, then assert those outputs match expected values; keep the metadata
checks that functionsByName.get(name)?.returnType === "Bool" but replace
fixturePasses(...) assertions with assertions against the actual fixture
execution results for "landscape_fixture", "portrait_fixture", "square_fixture",
"border_fixture", and "small_no_upscale_fixture" (use
selectExportSpec/planOutput outputs produced by the planner runtime to form the
expected values).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02c500c2-9cd4-4c67-8a44-fa88e5e0d07b
📒 Files selected for processing (2)
experiments/passepartout-layout-planner/src/main.0tests/passepartout-planner-fixtures.test.mjs
📜 Review details
🔇 Additional comments (3)
experiments/passepartout-layout-planner/src/main.0 (2)
19-36: LGTM!
93-142: LGTM!tests/passepartout-planner-fixtures.test.mjs (1)
87-109: LGTM!
fab5a58 to
5ed2479
Compare
435275e to
22febac
Compare
5ed2479 to
a7d38e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/branch-knowledge.md`:
- Line 29: Update the sentence in docs/branch-knowledge.md that currently states
"Passepartout now has named fixture functions for landscape, portrait, square,
and border planning" to either list all six fixtures (landscape, portrait,
square, border, no-upscale, oversized-border) or explicitly state it describes
the EDH-328 baseline and that EDH-330 later adds the no-upscale and
oversized-border probes; reference the document's mention of EDH-328/EDH-330 and
the term "Passepartout" so the change is made in the correct paragraph.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01a8af79-1b5a-46d1-aa35-4c087df24f9a
📒 Files selected for processing (4)
docs/branch-knowledge.mdexperiments/passepartout-layout-planner/README.mdexperiments/passepartout-layout-planner/src/main.0tests/passepartout-planner-fixtures.test.mjs
📜 Review details
🔇 Additional comments (8)
experiments/passepartout-layout-planner/README.md (1)
9-29: LGTM!docs/branch-knowledge.md (1)
32-32: ⚡ Quick winNo terminology correction needed—"export c" is the correct Zero syntax.
The documentation accurately reflects the actual codebase. The export modifier "c" is consistently used across all fixture functions and is the correct Zero language syntax, not shorthand for "export const" or any other form.
> Likely an incorrect or invalid review comment.experiments/passepartout-layout-planner/src/main.0 (5)
33-56: LGTM!
58-74: LGTM!
113-120: LGTM!
122-137: LGTM!
139-148: LGTM!tests/passepartout-planner-fixtures.test.mjs (1)
27-34: LGTM!
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
a7d38e3 to
eb22b69
Compare
b5b1b70 to
eff4d9a
Compare
eff4d9a to
caf3f7b
Compare
eb22b69 to
67229f9
Compare
caf3f7b to
c9cd8cf
Compare
67229f9 to
6ee3c59
Compare
Merge activity
|
c9cd8cf to
283f3f3
Compare
1 participant
Summary by CodeRabbit
Documentation
New Features
Tests