Skip to content

Bound native20 product-image conversion without losing source images - #405

Open
shiny-code-app[bot] wants to merge 5 commits into
work/opw314-batch10-1107from
work/opw314-images-1107
Open

shiny-code-app[bot] wants to merge 5 commits into
work/opw314-batch10-1107from
work/opw314-images-1107

Conversation

@shiny-code-app

@shiny-code-app shiny-code-app Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Website Sale’s native image conversion reads main/first-extra image content across the catalog. Those attachment reads memoize bytes in the ORM raw cache: about11GB for OPW’s16,319 templates, while image_vals contains only43 copies. The captured server/config startup implies Odoo’s default2,560MiB RLIMIT_AS; the container init process’s unlimited limit does not describe odoo-bin.

Call the unchanged native conversion in groups of32 templates, assert its first scoped search mentions image_1920, and flush/invalidate the cache after each group without committing. Native selection, contexts, image associations and transaction semantics remain. No memory increase, business-image shrinking, copied native algorithm or alternate no-op/post-hook scheme.

Classify new image copies above the saved19 maximum only with same-template/variant ownership, matching checksums and the same physical filestore path; the original attachment’s strict digest validates those shared bytes. Original and unrelated business attachments stay strict. Failed/blocked diagnostics prevent a completed20 result.

Validation on346328b:164 preservation tests pass;26 companion driver tests pass. The pinned20 Domain implementation proves the old source accepts a changed first search and the new assert rejects it, while valid native image search scopes/restores correctly. Persistent live ORM tests cover those paths; current-head CI is running. Ruff/format/comment/diff checks pass. IDE remains UNKNOWN under the existing allowance.

Weighed Anthropic Claude Opus5.5 reviews and the complete independent review https://github.com/cbusillo/odoo-tenant-opw/issues/474#issuecomment-6713645160 . Adopted its cause correction and shape guard; retained batch32, flush/invalidate and transactions. Earlier duplicate hashing, mirror fakes and connection findings are addressed.

Original run017’s frozen6005cc2 runtime/hook/request/manifest binding was proven on https://github.com/cbusillo/odoo-tenant-opw/issues/314#issuecomment-6713718057 . It was cancelled before intake through the supported runner:143/cleanuptrue/no staged inputs or Odoo. Its frozen packet is preserved. Corrected017 is preparing from the same completed19 pair/image/framework; no completed20/testing restore is earned. #403 alone does not contain this image fix.

Stacked on #403; original branch/tree/evidence remain.
Refs #404
Refs cbusillo/odoo-tenant-opw#314

Removed: full-catalog raw-cache retention, copied native algorithm, nontransactional image journal, mirror fakes, duplicate hashing and leaked fixture connections. Net lines:+192 from b17544f; latest search-shape guard/regression/docs+10 (+11/-1).

Client test notes

Nothing for the Client to test: this is shared offline migration preparation. It bounds memory while Odoo 20 converts Website Sale product images during migrated-copy loading, and keeps every original image and attachment. It adds no screen, wording or day-to-day workflow. OPW staff workflow testing remains on the actual migrated testing result tracked by the parent.

@shiny-code-app

shiny-code-app Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 6005cc2

Weighed the completed Anthropic Claude Opus5.5 reviews against current6005cc2. The revised review examined cd1540c plus selected native method, caller, manifest and loader; it found no high/medium defect. Adopted its low findings by removing duplicate hashing in favor of exact shared-path identity plus the original attachment digest, deleting mirror unit tests in favor of real ORM/SQL checks, and closing fixture connections. Corrected a CI fixture’s creation-order assumption to compare the actual native query order. Focused local tests and current live19/live20 CI pass.

The later website_sale update concern is outside this cross-version hook’s current execution; it does not establish a current defect or a new migration finish line. The OPW driver keeps exact stock portal aliases and strict original/unrelated business attachments. All observed run016 failures remain reported; run017 is the next single consolidated saved19-only attempt. This review is source evidence, not migrated-data or testing-restore proof.

Removed: copied native conversion, rollback-surviving journal, mirrored fakes and duplicate byte hashing. Net lines:+182.

@shiny-code-app

shiny-code-app Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 346328b

Weighed the complete independent review at https://github.com/cbusillo/odoo-tenant-opw/issues/474#issuecomment-6713645160 . Corrected cause: attachment content reads memoize bytes in ORM raw cache (about11GB for OPW); image_vals contains only43 copies. The2,560MiB RLIMIT attribution is inferred from captured server/config initialization, not the init process’s unlimited limit. Kept the unchanged native method, batch32, flush_all/invalidate_all and transaction semantics. Added the requested one-line first-search image_1920 assertion and real ORM regression for a changed search shape. The actual pinned20 Domain implementation accepts the valid scoped domain and rejects the changed domain; baseline6005 did not reject it.164 preservation tests and lint/format/comment checks pass. Current-head CI is running; IDE stays UNKNOWN under the existing allowance.

Original017 was cancelled through the supported runner before intake, terminal143/cleanuptrue/no staged inputs or Odoo. Its4056005 packet was proven byte-equal; #403 is not image-fix proof. Its frozen inputs remain intact. Corrected017 will use the same completed19 pair/image/framework with fresh immutable preparation and no replay of an uncertain operation. No alternate no-op/post-hook scheme, memory increase, image shrinking or new qualification round.

Removed: silent acceptance of a changed native first-search shape. Latest Net lines:+10 (+11/-1); image change total+192 from b17544f.

@shiny-code-app

shiny-code-app Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

Exact-source review for head 346328bae8d791f1de714cdd23f7bd0757894c77: Claude Opus5.5 reviewed the implementation and native callers, no actionable findings. Weighed; optional test-name wording nit declined because the explicit assertions are clear. No additional review or source round.

Current head passes all4 CI checks, including native19/20 live checks. Both consumer builds pass: OPW42420561278 and website42420563054, with retained manifest ZIP digests and exact devkit source bindings verified. Focused164 preservation tests and real pinned20 Domain-library failure/restoration probe passed. IDE remains UNKNOWN under the existing allowance; no clean IDE claim.

Corrected017 source/file hashes were published before submission; actual original request binding is queued from the completed19 pair. Native migration success/testing restore are still unearned. This PR contains the image fix; #403 alone does not.

Removed: silent changed-search acceptance; prior image batch work removes full-catalog attachment raw-cache retention while preserving native conversion and transactions. Latest delta Net lines: +10 (+11/-1); image work net+192 from its original parent.

@shiny-code-app

shiny-code-app Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

Review result: weighed
Head: 346328b

Weighed the exact-source review: Anthropic Claude Opus5.5 on claude-icloud, nativef7c2f196, reviewed this unchanged346328bae8d791f1de714cdd23f7bd0757894c77 image delta against6005cc2. No actionable findings. Declined the optional test-name wording change because the assertions are unambiguous; no source/test/review round for wording.

Corrected017 received these exact bytes and progressed past the prior image/data.xml memory failure to a later unrelated action-path conflict. Its bounded-loader check passed. The complete migration still failed; this review and source CI do not establish migrated data. No duplicate review was launched, and this record does not claim review coverage for the separate new action delta in #407.

@shiny-code-app
shiny-code-app Bot marked this pull request as ready for review November 8, 2026 01:48
@shiny-code-app

shiny-code-app Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

Ready for the merge train

Head: 346328b

@shiny-code-app

shiny-code-app Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor Author

Launchplane is waiting before the next merge-train step.

waiting for stack parent #403 before landing on main. Unrelated pull requests can proceed.

Launchplane manages this comment and will update it as the train moves.

@shiny-code-app

Copy link
Copy Markdown
Contributor Author

The current train stop is staffed. The source finisher is registered on owning issue 404 as native e9d54140-a0bc-499e-9b52-44ddaca36b72, using claude-icloud and Opus 5.5 high effort under Odoo generation 9.

It owns the failing current-head required checks and missing Client test notes here, then the existing parent/child source and CI propagation for PRs 403 and 407 through the normal checked Ready and train path. The retained source claim and authored release are verified before its own task worktree. Older green CI and Ready comments remain historical; this comment does not claim the current checks pass.

The stack remains held by the actual check and notes failures until the worker earns current-head eligibility or records the exact remaining external blocker. No manual merge, stack collapse, data rerun, lane change or production action is assigned. Source progress and the final result return on issue 404.

@shiny-code-app

Copy link
Copy Markdown
Contributor Author

Ready for the merge train

Head: 346328b

@shiny-code-app

Copy link
Copy Markdown
Contributor Author

Current-head evidence for 346328b, unchanged source.

Cause of the train stop. All 4 current-head check runs on this head are green; none ever failed. The PR body had no Client test notes. Launchplane's snapshot sets a PR's required-check status to fail whenever the test-notes parse has an error (control_plane/merge_train_github.py, ("fail", "") if review_intent.error). That one gap produced both of the train's reasons. The shared parser reproduced Client test notes are missing. on the old body and returns no error on the new one.

Fix. Added ## Client test notes with Nothing for the Client to test, matching 403 and 407: this is shared offline migration preparation with no screen, wording or workflow change. No source change, push, rebase or retarget.

Stack. This head is based on b17544f, not 403's head 0fcf6f0. The two changes touch no common file, and main+403 and 403+407 merge cleanly. The train builds and checks its own combined candidate, so the branch was not refreshed only for being behind.

Native inspection. Real isolated native PyCharm run on this exact head's 3 changed Python files: RED, 16 findings on changed lines (33 preexisting not counted). I read every site. None is a source defect:

  • 4 unresolved odoo references in docker/scripts/product_images_20.py, and 9 _SupportsClose cursor/commit/rollback findings in the live test. The inspection interpreter lacks the Odoo runtime. Untouched code shows the same No module named 'psycopg2' and unresolved odoo findings, and psycopg2's connection type is lost through closing(). These run inside the Odoo image, where live 19 and live 20 CI pass.
  • Unused cr in pre-product-images.py: Odoo's migrate(cr, version) signature requires it.
  • product_images_20.py:58 return type object vs None: a narrowing artifact of callable(getattr(...)); the wrapper returns None.
  • A weak shadowing warning at test line 171 belongs to 403's commit b17544f.

Tests. Locally on this head: 228 tests across test_odoo_data_workflows (134), test_core_references_20 (55), test_payment_20 (17), test_migration_diagnostics, test_sale_20, test_upgrade_paths, test_docker_script_override_guards and test_core_action_20 pass. Ruff check and format pass. The full suite and live 19/20 checks pass in current-head CI.

Carried, unchanged. The weighed exact-source review for this head and the consumer builds OPW 42420561278 and website 42420563054 still bind this exact head. No new review round, since no source changed.

Checked Ready for this head: #405 (comment) . 403 and 407 were re-verified on their current heads too.

@shiny-code-app

Copy link
Copy Markdown
Contributor Author

Hold on the 403/405/407 stack, read from the train's pass at 11:34 AM ET.

405's own stop is fixed. The train no longer reports failing checks or missing notes on 405, and 403, 405 and 407 each have checked Ready on their current heads (0fcf6f0, 346328b, bc3bb03).

The stack still cannot land. Draft #411 is stacked on 407, and the train collapses every stacked child into the root. 403 is therefore blocked with stack_unsupported: #411 is a draft and has no ready-to-merge label. 411 is issue 410's released source, and it is outside this finisher's assignment.

Next actor: agent:supervisor-milestone, the recorded next actor on #410 , through the Odoo lead. Choose one:

  • The documented stacked-PR route: retarget held draft 411 to main and leave it draft. The train then lands 403/405/407, and 411 continues on its own.
  • Finish 411's proof and Client test notes, then mark it checked Ready so the whole stack lands together.

Never ready or label 411 just to unblock its parents. No base, branch or source here was changed.

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.

0 participants