Repository navigation
Conversation
Resolve R-01 by annotating the image conversion helpers, serverless Image.save, the browser metadata context manager, and a named Chromium launch callback. Preserve call-time timeout calculation, headless launch, and executable-path forwarding. Resolve R-03 with DEBUG diagnostics for incomplete browser executables and unknown registry revisions, preserving all caller-facing exception messages and launch selection. Resolve R-04 with concrete success and None examples. Document DEBUG diagnostics in both browser-PDF guides. Verified on Windows with ADR 27.1 builds 1248 and 1223: 194 focused existing tests passed; matching, mismatched, leftover, incomplete, and malformed-registry runtime scenarios passed. Pre-commit (documented Windows license-hook skip), make check, make smoketest, and make docs passed; docs emitted 504 warnings. Changed functions have complete annotations; unrelated annotation findings and two pre-existing unused imports remain out of scope. R-02, R-05, R-06, test authoring, and final coverage remain deferred to phase 09. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the explicitly approved post-commit dependency refresh. Update click 8.5.0 to 8.5.1, python-discovery 1.9.4 to 1.9.5, ruff 0.17.3 to 0.17.4, and virtualenv 21.15.5 to 21.15.6. Retain uv-generated lockfile revision and default-groups metadata. No packages were added or removed, and runtime dependency versions are unchanged. Verified the refreshed environment after the review-fix commit with pre-commit (documented Windows license-hook skip), make check, make smoketest, and the same three focused existing test files against ADR 27.1. Workflow artifacts and generated test data are excluded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #668 +/- ##
==========================================
+ Coverage 82.08% 82.09% +0.01%
==========================================
Files 32 32
Lines 9443 9395 -48
Branches 1698 1689 -9
==========================================
- Hits 7751 7713 -38
+ Misses 1195 1189 -6
+ Partials 497 493 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
Browser-PDF export now launches the headless shell named in the ADR installation's browser metadata by its executable path on every export. Before, it read Playwright's undocumented driver/package/browsers.json to find the build this Playwright version expects, launched by path only when the ADR build differed, and otherwise let Playwright's own lookup choose a folder. Launching by path works for every build, so the comparison only decided whether to log a mismatch warning. It also kept a fallback where Playwright's lookup could pick a leftover build folder or leave temporary folders behind when browsers.json could not be read (#667). Remove _expected_headless_shell_build, _launch_packaged_browser, _HEADLESS_SHELL_BUILD_PATTERN, and the private playwright package import. Drop the mismatch warning, which no longer describes a behavior change. Keep the pre-launch check for a missing executable, since a failed launch leaves Playwright temporary folders behind, and keep launch failures unretried with the original Playwright error as their cause. Type the launch logger as logging.Logger, which get_logger() returns. Launching by path skips Playwright's host-dependency check (confirmed in the Playwright 1.64.1 driver), so a missing Linux system library surfaces as a launch failure; the docstring records this trade-off. Update the browser-PDF guides, CHANGELOG, and the Playwright pin comment to remove the browsers.json DEBUG records and the mismatch warning. Replace the comparison tests with by-path launch tests for matching, different, and non-revision builds on both platforms, missing executables, and unretried failures. Verified on Windows: 155 focused tests pass against ADR 27.1; real renders with ADR builds 1248 and 1223, a leftover build, and an incomplete build behave as expected and leave no Playwright temporary folders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Browser-PDF export no longer sets PLAYWRIGHT_BROWSERS_PATH to the ADR browser folder or clears PLAYWRIGHT_HOST_PLATFORM_OVERRIDE while it runs. Playwright reads PLAYWRIGHT_BROWSERS_PATH only to find, install, or list browsers, and the host-platform override only affects which executable its lookup chooses. Now that every export launches the ADR browser by its executable path, neither variable affects the launch. Changing them was still process-wide, which is why both browser-PDF guides told users to run one export at a time in each Python process. Remove _playwright_browser_binary_env, _TRANSIENT_PLAYWRIGHT_OVERRIDE_ENV_VARS, and the imports only they used. render_pdf resolves the browser package first, inside the same try block, so a missing or unsupported package raises the same ADRException. Log the browser folder there, and drop the generic "Launching headless Chromium" INFO line, which repeated the following "Launching Ansys installation browser build" line. Remove the one-export-per-process paragraph from both guides and add a CHANGELOG entry, since rc4 shipped the environment changes. Replace the environment-scoping tests with one test that the variables are unchanged at Playwright startup and after the render, whether set or unset, and drop the module fixture that set PLAYWRIGHT_BROWSERS_PATH for real-browser tests. Verified on Windows: 153 focused tests pass against ADR 27.1; real renders with ADR builds 1248 and 1223 succeed with a bogus PLAYWRIGHT_BROWSERS_PATH and a Linux host-platform override set, and leave both unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The browser-package checks in _validate_playwright_browsers_path logged their warnings through get_logger(), the package logger, instead of the logger the export was created with. A caller that passed its own logger never saw that leftover browser builds were ignored or that the browser metadata was missing, unreadable, or incomplete. _validate_playwright_browsers_path now requires a logger, so no caller can omit it. The public resolve_playwright_browser_binary_info gains an optional logger argument that falls back to the package logger, keeping existing calls unchanged, and the renderer passes its own logger. Add tests that a leftover-build warning and a rejected-package warning go to the given logger rather than the package logger, and that the renderer forwards its logger to the resolver. The existing leftover-build test still covers the package-logger fallback. Verified on Windows: 155 focused tests pass against ADR 27.1, and a real render with a leftover build reports the ignored build on the renderer's logger with no records on the package logger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run uv sync --upgrade --all-extras and uv lock --upgrade after the browser-PDF commits, per the repository workflow, so the Security Scan check runs against current locked versions. The only change is cyclopts 5.3.3 to 5.4.0. Verified: make smoketest passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The browser-package check listed the ADR browser folder on every export and logged a warning naming any build folders that the metadata does not name, for example after copying a newer ADR package over an older install. Now that every export launches the metadata-named build by its executable path, a leftover folder is never read or launched, so the warning described something harmless and repeated on every export. Its advice to clean the installation belongs to the ADR installer, not PyDynamicReporting. Leftover folders are still ignored and still don't cause the browser package to be rejected. The warning was added after rc4, so removing it is not a user-visible change from the last release. Remove the listing and warning. The leftover test now asserts that the package is accepted without a warning. The logger-routing test that used the leftover warning becomes a test that warnings fall back to the package logger when no logger is given; the rejected-package test still covers the given logger. Update both browser-PDF guides and the CHANGELOG. Verified: 155 focused tests pass against ADR 27.1; pre-commit, make check, and make smoketest pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_packaged_headless_shell_executable returned None both when the platform had no known executable layout and when the executable was missing, so _launch_product_browser repeated the _PACKAGED_HEADLESS_SHELL_EXECUTABLES lookup only to choose its DEBUG message. The helper now takes the logger, looks the layout up once, and logs the missing-executable record itself; _launch_product_browser just raises the existing ADRException. The unknown-platform branch is removed: the resolver only accepts metadata whose machine_arch matches _playwright_machine_arch(), which returns only win64 or linux_2.6_64, the table's only keys. The caller-facing error and the DEBUG message text are unchanged. Drop the "unsupported" case from the missing-executable test and pass the logger through the test stub. Verified: 154 focused tests pass against ADR 27.1; pre-commit, make check, and make smoketest pass; real renders on Windows succeed with build 1248 and raise the same ADRException with the DEBUG record for an incomplete build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… callback _validate_playwright_browsers_path rejected a metadata-named build folder that was missing, and then rejected the same folder again when its INSTALLATION_COMPLETE marker was missing. A missing folder always fails the marker check, so the first check only changed the warning text. Remove it; the marker warning names the full marker path, which includes the folder. The check that the metadata names a direct child folder stays, since it keeps the metadata from pointing outside playwright-browsers. render_pdf's launch_chromium accepted arbitrary keyword options, but it is only ever called with executable_path. Take that keyword explicitly. Verified: 154 focused tests pass against ADR 27.1; pre-commit, make check, and make smoketest pass; real renders on Windows succeed with build 1248, and metadata naming a missing folder still raises the same ADRException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Browser-PDF export launches the ADR browser by its executable path, which skips Playwright's host-dependency check. A missing Linux system library therefore surfaces as a browser startup failure rather than Playwright's "Host system is missing dependencies" message. Playwright also skips that check when the build folder has a DEPENDENCIES_VALIDATED marker under 30 days old, which ADR ships, so the old launch did not always run it either. Users had no way to learn the requirement from the guides, so state it in the browser requirements of both browser-PDF guides. Verified: pre-commit passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert c0de7b5. Supplying the browser's Linux system libraries is the responsibility of ADR packaging and installation, not of PyDynamicReporting users, so the browser-PDF guides should not ask users to provide them. ADR packages the browser with "playwright install chromium --only-shell", which does not install or bundle operating-system libraries, and the ADR 27.2 Linux archive does not ship them. The gap is tracked on the ADR packaging side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
On Linux the headless shell needs operating-system libraries such as NSS, GLib, GBM, ALSA, ATK, and X11. ADR packages the browser with "playwright install chromium --only-shell", which does not ship them, and asking users to install them is not acceptable. ADR will ship them in the runtime_libs folder next to playwright-browsers instead. The packaged chrome-headless-shell has no RPATH or RUNPATH, so the dynamic loader finds those libraries only through LD_LIBRARY_PATH. The browser is started from the user's Python process, not ADR's launch scripts, so PyDynamicReporting must supply it. _browser_library_env passes the browser a copy of this process's environment with runtime_libs prepended to LD_LIBRARY_PATH, using Playwright's launch(env=...). Playwright replaces the browser's whole environment with the one given, hence the full copy. The Python process's own environment is untouched, so exports still don't change process-wide state. Windows, and Linux installations without a runtime_libs folder, keep Playwright's default environment. Add tests for the Linux library path with LD_LIBRARY_PATH set and unset, that the rest of the environment is carried over and os.environ is unchanged, and that Windows and a missing runtime_libs keep the default. Verified on Windows: 158 focused tests pass against ADR 27.1 with every changed line covered; pre-commit, make check, and make smoketest pass; a real render still succeeds. Not yet verified on Linux: Docker is not running here, and ADR does not ship the libraries in runtime_libs yet. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert d058386. Customers already run browser-PDF export on Linux with rc4 and ADR 27.1, so the browser's system libraries are present on their machines. Prepending ADR's runtime_libs to the browser's LD_LIBRARY_PATH would add an untested change to a working setup: that folder already holds ADR's own libssl, libcrypto, libz, and an NSS module that could shadow the system NSS the browser loads. Revisit bundling only if a customer reports a missing library; the export error keeps Playwright's original error, which names it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address R-02, R-05, R-06 and TA-1 from the browser-export test audit for #667. Exercise ADR.create_item(Image) and the real Image.save conversion with signed 32-bit TIFF pixels stored under a JPEG filename; assert the persisted 16-bit PNG clamps both extremes and delete the created item afterward. Replace the render-stack private executable-lookup patch with a temporary packaged executable and the public resolver seam. Preserve independent path assertions, correct the no-launch test name and metadata comment, and annotate changed and explicitly listed tests and helpers. Verified 159 focused tests against local Windows ADR 27.1, direct changed-line coverage of 30/30 with no partial changed branches, scoped annotation and unused-code checks, pre-commit, make check and make smoketest. A real public ADR browser export produced a one-page PDF with verified content and an unchanged environment. The license-header hook was skipped as explicitly authorized. No production, documentation or dependency changes are included. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run the required uv sync --upgrade --all-extras and uv lock --upgrade after the focused test correction commit. Package versions are unchanged; current uv canonicalizes the lockfile to revision 5 and records the existing all-default-groups setting. Verified uv lock --locked, pre-commit (with the explicitly authorized license-header skip), make check, make smoketest and 159 focused tests against local Windows ADR 27.1. Keep this authorized lockfile refresh separate from the test-only change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rowser On Linux the headless shell loads operating-system libraries that ADR does not ship, such as NSS, GLib, GBM, ALSA, ATK, and X11. Typical ADR hosts have them, which is why customers run browser-PDF on Linux with rc4 and ADR 27.1, but a minimal server or container may not. Record this in the docstring of the function that launches the browser, together with what to look for: the Playwright error chained to the ADRException includes the browser's stderr, so the loader's "error while loading shared libraries" line names the missing library. Note that Windows needs nothing extra. Verified: pre-commit and make smoketest pass; focused renderer tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a short paragraph to the browser requirements of both browser-PDF guides. Windows always provides the system libraries the browser needs. Most Linux machines that run ADR do too, but a minimal Linux server or container can lack some, and the export then fails when the browser starts. Tell users how to find the missing library (DEBUG logging shows the browser's "error while loading shared libraries" line), to install the system package that provides it or ask an administrator, and point to Playwright's system dependency documentation, which describes installing them all at once on supported distributions. Verified: pre-commit passes; make docs succeeds with no warnings from the edited pages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Right after telling users to set the ansys.dynamicreporting.core logger to DEBUG, show the two-line get_logger(log_output="stdout", log_level="DEBUG") call from the logging guides, and link to the matching logging page. It works for both serverless and service-mode exports, since both use the package logger. Verified: the snippet alone prints the "Browser PDF rendering failed." DEBUG record with the browser's stderr lines for a failed launch; pre-commit passes; make docs succeeds with no warnings from the edited pages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Show the log_output and log_level arguments of ADR and Service instead of calling get_logger directly. Users already construct those objects in these guides, the constructor arguments are the primary documented way to configure logging, and they set the same package logger the browser-PDF renderer writes its DEBUG failure record to. Verified: pre-commit passes; make docs succeeds with no warnings from the edited pages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The comment on the playwright pin said browser-PDF launches the packaged build by path, "so" a pin change needs testing, which does not follow. Launching by path is why any ADR browser build starts regardless of the pin. The reason to test is that a newer Playwright may not be able to control an older browser build, so state that instead. Verified: pre-commit and make check pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The note told users to enable DEBUG logging for a record naming a missing browser executable. The ADRException already names the incomplete browser build, so the record only adds the full path, which matters to support rather than users. The browser requirements section now explains how to enable DEBUG logging for the case where it does help, a missing system library. The DEBUG record itself stays in the code. Verified: pre-commit passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
resolve_playwright_browser_binary_info fell back to get_logger() when no logger was given, but its only caller is the browser-PDF renderer, whose logger is already the caller's logger or get_logger(). The function is not exported from the package or documented, so the second fallback had no user. Make logger a required keyword argument and pass it straight through. Remove the test of the package-logger fallback, pass a logger in every other call, and have the leftover-build test check its own logger instead of patching get_logger. Verified: 153 focused tests pass against ADR 27.1; pre-commit, make check, and make smoketest pass; a real Windows render succeeds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- The renderer constructor already rejects a missing ansys_installation or ansys_version and expands the install path. Stop converting that Path back to a string, and stop re-checking both for None and expanding the path again in resolve_playwright_browser_binary_info, whose only caller is the renderer. Both arguments are now required. - Fold _browser_pdf_product_line and _browser_pdf_product_release into _raise_if_product_line_unsupported. The release was computed twice, the None check on the version repeated the constructor's, and the "Product release information could not be determined" ValueError could never be raised, because a known product line implies a known release. - json.JSONDecodeError is a ValueError, so drop it from the except tuple. - Remove _PlaywrightBrowserBinaryInfo.to_metadata_dict, which only a test helper used; the helper now builds the metadata dict itself. Remove the test that passed None installation or version to the resolver, expect the Path in the resolver-forwarding test, and add a test that an install version without a known product release is not rejected as too old. Behavior and error messages are unchanged. Verified: 153 focused tests pass against ADR 27.1 with every changed line covered; pre-commit, make check, and make smoketest pass; a real Windows render succeeds with ADR 27.1, and ADR 26.1 is still rejected with the same message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_browser_binary_info and _headless_shell_binary_info built the same _PlaywrightBrowserBinaryInfo, one with fixed values and one with parameters. Keep _browser_binary_info with optional packaged-folder and machine-arch arguments and use it everywhere. Verified: 153 focused tests pass against ADR 27.1; pre-commit passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tence - Remove test_resolve_playwright_browser_binary_info_rejects_missing_named_dir_beside_leftover. It builds the same scenario as the existing ..._rejects_packaged_dir_name_mismatch: one folder on disk, metadata naming a different one, expect None. The leftover listing it guarded is gone. - Remove test_playwright_pdf_ignores_another_complete_build_beside_the_metadata_selected_build. It stubs the resolver, so the extra build folder it creates is never read by the code under test; it asserted the same launch path as test_playwright_pdf_direct_launch_logs_build_and_version. Leftover handling is covered by the resolver tests. - Remove _arrange_revision_build_renderer. It only gave the build a revision-style name, which mattered for the removed build comparison. Its remaining user now uses _arrange_product_browser_renderer. - Collapse the build-name parametrization of the launch test to one name: with no build comparison, "different", "matching", and "non-numeric" builds take the same path. Both platforms stay. - Use DOCKER_DEV_REPO_URL in tests/test_compatibility.py instead of repeating the image URL, like every other test; #665 had to edit both copies by hand. - Trim the CHANGELOG environment-variable entry to its first sentence; the Fixed entry already says the browser launches by path. No production code changes. Verified: 147 focused renderer tests and 37 compatibility tests pass against ADR 27.1; pdf_renderer.py coverage is unchanged at 96.18% with no newly uncovered lines; pre-commit, make check, and make smoketest pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove four logger.warning.assert_not_called() assertions. Every warning in pdf_renderer.py is in _validate_playwright_browsers_path, which none of these tests reach: three call _launch_product_browser directly, and the renderer test replaces the resolver. They were left over from the removed build-mismatch warning. - Merge test_resolve_playwright_browser_binary_info_finds_full_install_layout and ..._accepts_metadata_matching_packaged_dir. They had identical setup and the first asserted a subset of the second; keep the first name with the full assertions. - Rename ..._rejects_packaged_dir_name_mismatch to ..._rejects_metadata_naming_a_missing_dir, with its constant, since the single-folder "mismatch" rule it was named after no longer exists. - Trim the renderer launch-logging test to what only it checks: that render_pdf logs the launch through the renderer's logger. Three other renderer tests already assert the executable_path wiring. - Replace "point Playwright at a product-managed binary" in the _validate_playwright_browsers_path docstring; the export launches the binary by path and no longer redirects Playwright. Reviewed and left unchanged: the renderer constructor docstrings (still accurate), the separate marker and executable checks (they detect different damage, and merging them changes a documented error message), the resolve/validate split, the platform names in two tables (different facts), and the equal Docker image constants (pre-existing public API). Verified: 146 focused tests pass against ADR 27.1; pdf_renderer.py coverage is unchanged at 96.18% with no newly uncovered lines; pre-commit, make check, and make smoketest pass; no references to the removed names remain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Group direct headings and optional introductions with their template-owned content in basic, item-based box, header, tag-property, and footer layouts. Preserve positioned layouts and explicit footer page breaks, and keep oversized content splittable. Add generator-shaped PDF regressions for custom sections, optional report parts, nested layouts, oversized content, and footer breaks. Update both browser-PDF guides and the changelog. Verified 247 focused PDF tests on each ADR 271 and 272 installation, pre-commit, make check, and make smoketest; the Windows license-header hook was skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run the required post-commit uv refresh. Update filelock, importlib-metadata, markdown-it-py, and soupsieve; include the lazy-importer dependency required by importlib-metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
Account for panel titles and spacing through tabs and other nested layouts so A4 landscape output does not split a panel title from its first visual or introduce blank pages. Keep multi-section panels splittable and avoid reserving their header space for every image. Add 36 real-browser regression cases for wrappers, nested panels, optional headings, page formats, and multi-section content. Update both browser-PDF guides and the changelog. Verified 283 focused PDF cases on each of ADR 27.1 and 27.2, 96.19% Python renderer coverage, real-report A4/A3 comparisons, local diagnostics, pre-commit, make check, and make smoketest. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
viseshrp
changed the title
…ripts The visual-fitting script and the cohesion script each walked from a layout's heading to the section.adr-container whose id equals the layout's nexus_template value. The two copies had to stay identical, or the passes would disagree about which container a layout owns (review finding RV-06). The walk now lives in one module-level JavaScript snippet, _PAGINATION_FIND_LAYOUT_CONTAINER_JS, which defines findLayoutContainer and is substituted into both scripts through a placeholder, the same way _apply_pdf_capture_styles substitutes __ANSYS_VIEWER__. Behaviour is unchanged. Verified with tests/serverless/test_pdf_renderer.py and tests/serverless/test_pdf_renderer_browser_binary.py: 283 passed. The real-browser cases ran against Playwright's Chromium headless shell build 1243 as a stand-in, because this machine has no ADR 27.x installation. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Browser-PDF export launches the ADR browser by a path built from _PACKAGED_HEADLESS_SHELL_EXECUTABLES. That inner folder layout is written by Playwright's installer and is not a documented Playwright interface (review finding RV-05). The comment on the table now states where the layout comes from (the Playwright release that built the ADR browser, chromium_headless_shell-1248 for 1.64.1), that Playwright does not document it, that a product build with another inner layout makes every export fail as an incomplete build, and that the keys must match the values _playwright_machine_arch returns. A second comment at the lookup states why indexing the table without a fallback is safe. Comments only; no behaviour change. Verified: ruff format --check and ruff check pass on the file, and tests/smoketest.py exits 0. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Two regressions from main in the browser-PDF visual-fitting script. RV-01: includePanelBounds granted the panel header only to the panel's first visual element, and only when the panel's first item contained it. The bottom spacing had the mirrored rule. A section with a table before an over-height image, or one item holding two over-height images, was therefore fitted as if the panel header did not exist. The panel then exceeded the page and its title was left behind, sometimes after a blank page. The helper now decides from the group its caller passes in. It widens the group to the panel top when no visible item in the panel starts above the group, and to the panel bottom when none ends below it. Every visual in the first or last group gets the same budget. Items are kept in document order and searched from the matching end, so the whole list is scanned only for visuals in a panel's first or last group. The cached per-panel visual list is no longer needed and is removed. RV-02: a layout with a heading claimed every visual below its container, including visuals owned by nested layouts that have their own heading. Only the first owner got the layout top; later nested visuals were fitted from their own item top, which left out the nested heading, so that heading was split from its visual. The widened layout selector and the introduction support made more parents match than on main. The loop now resolves all heading layouts first and skips a visual that the nearest layout-owned container assigns to a nested layout. That nested layout fits it in its own iteration with its own heading in the budget. The parent still fits its first owner, nested or not, so the parent's heading and introduction stay in that visual's page as they do on main. Verification, on Windows 11 with Playwright 1.64.1: - tests/serverless/test_pdf_renderer.py and tests/serverless/test_pdf_renderer_browser_binary.py: 283 passed. - The REVIEW.md reproductions, rendered through the real renderer on A4 landscape, before and after. RV-01 A with a section heading: 4 pages with a blank page and a panel of 825 px before; 2 pages and a 710 px panel inside the 710.3 px budget after. RV-01 B: both images now get the same height and the panel fits. RV-02: CHILD-2 now shares a page with its image. - This machine has no ADR 27.x installation. The real-browser cases above ran on Playwright's Chromium headless shell build 1243 as a stand-in. The run on the ADR browser is CI's Linux Testing jobs. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…eased The branch had rewritten the released "Browser-PDF layout, pagination, and output" entry under 1.0.0rc3 to describe heading and introduction grouping in basic, box, header, tagprops, and footer layouts. That behaviour is not in 1.0.0rc3 or 1.0.0rc4, and Unreleased had no entry for it (review finding RV-04). The 1.0.0rc3 entry is restored to its text at tag v1.0.0rc4, and Unreleased gains one Fixed entry for the grouping. Verified: git diff v1.0.0rc4 -- CHANGELOG.md shows a single hunk, above the 1.0.0rc4 heading. codespell and the whitespace hooks pass. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
uv lock --upgrade moves cyclopts 5.4.0 to 5.4.1, a transitive development dependency. No runtime dependency changes. Verified with uv lock --locked (exit 0). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
The text this branch added to the two browser-PDF guides and to the Unreleased changelog described pagination in terms of the renderer's internals: content containers, grouping steps, layout wrappers, positioned box children, content fragments, and browser metadata. A reader exporting a report does not need those terms. Guides (serverless and service mode): - The pagination paragraphs become a short list of what stays together on a page: a heading with its section, a panel title with the panel's first content, and over-height visuals scaled to fit. - The Linux system-library text is split into what can go wrong, how to turn on debug logging, and what to look for and install. The examples use Linux paths, because the text is about Linux. - The note on extra browser builds no longer mentions browser metadata. Changelog: the Unreleased browser-PDF entries state the effect for the user and drop launch and layout internals. No behaviour is described that the code does not have. Verified: sphinx-build -b html on doc/source exits 0 and builds both browser_pdf pages; blacken-docs, codespell, and the whitespace hooks pass. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Reserve the shared ancestor heading and introduction for the first content fragment in each first-row column, including nested grids. Keep later rows, wrapped columns, and continued sections independently sized. Add 30 real-browser regression cases and narrow the changelog claim about temporary folders left after failed browser starts. Verified both focused PDF modules: 313 tests passed on ADR 27.1 and 27.2, with 96.20% renderer coverage. Pre-commit, make check, and make smoketest passed with the approved Windows license-hook skip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
Supported nested multi-column layoutADR's columns can contain whole child layouts, not just individual items. For example: That renders as: Checked against ADR core's This is a supported generator-produced hierarchy, not something requiring hand-written grid HTML. The previously reported gap described the behavior before the latest fix: the shared parent heading's space must be included for the first content fragment in each first-row column, not just the first column. Later rows and sections retain their own available space. The pagination fix is committed and pushed as 2214651. 313 focused PDF tests passed on each of ADR 27.1 and ADR 27.2. |
Collaborator
Author
First-row column that starts with text: image shrinks to a sliverGPT Sol agent, this is a follow-up on 2214651. The fix is correct for the nested multi-column layout. It also changes one neighbouring shape for the worse. For example: Column 2's image, A4 landscape, 710 px page budget:
Cause: the loop at This is not new in kind. A single-column layout with 400 px of text above its first image gives 198 px both before and after the commit. The commit extends that rule to every first-row column. Requested change: when the heading budget would shrink a visual below a minimum, fit it against its own group instead, so it moves to the next page at full size. Apply one rule to the first column and to the other first-row columns. Please propose the minimum from what looks right in the customer reports and state it in your reply. Please keep the 30 new cases passing, add cases for text above the first image in column 1 and in column 2 (one above and one below the minimum), and run both PDF modules on ADR 27.1 and 27.2. Measured on a stand-in Chromium headless shell, not the ADR build. 313 focused PDF tests pass there on 2214651, and 14 of the 30 new cases fail on the pre-fix renderer. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes https://ado.internal.synopsys.com/tfs/ANSYS_Development/Portfolio/_workitems/edit/1539469
Continuation of #665.
Fixes #667.
Changes
Image.save, and the browser launch path; DEBUG diagnostics for incomplete browser builds; docstring examples.driver/package/browsers.jsonand Playwright's own browser lookup are removed, so a leftover build folder can never be launched and a failed lookup can't leave temporary folders behind.PLAYWRIGHT_BROWSERS_PATHandPLAYWRIGHT_HOST_PLATFORM_OVERRIDEare no longer set or cleared. A by-path launch doesn't read them, and changing them was process-wide, which is why the guides limited exports to one at a time per process. That limit is removed from both guides.resolve_playwright_browser_binary_infogains an optionalloggerargument and falls back to the package logger, so existing calls are unchanged.executable_path.Differences from the fix suggested in #667
_expected_headless_shell_buildis removed. The issue suggested keeping thebrowsers.jsonread just to warn when the ADR build differs from Playwright's. Since every build launches by path, the warning no longer describes a behavior change, and keeping it would keep the dependency on a private Playwright file.PLAYWRIGHT_BROWSERS_PATHscoping is removed rather than kept. The issue called it harmless, but it was the only reason exports had to run one at a time per process.Trade-off
Launching by path skips Playwright's host-dependency check, which only runs when Playwright looks up the browser itself (confirmed in the Playwright 1.64.1 driver). On Linux, a missing system library therefore shows up as a plain browser launch error instead of Playwright's "Host system is missing dependencies" message. The original Playwright error is still the cause of the ADR error. Playwright also skips that check when the build folder has a
DEPENDENCIES_VALIDATEDmarker less than 30 days old, which ADR ships, so the previous launch didn't always run it either. The libraries themselves were always required on Linux and are already present on customer machines running rc4 with ADR 27.1, so nothing is bundled. If one is ever missing, the ADR error's cause names it.Verification
tests/serverless/test_pdf_renderer.pyandtests/serverless/test_pdf_renderer_browser_binary.py, 155 passed against a local ADR 27.1 install; every changed line inpdf_renderer.pyis covered.pre-commit,make check, andmake smoketestpass.PLAYWRIGHT_BROWSERS_PATHand a Linux host-platform override set, leave both unchanged, and leave no Playwright temporary folders.