Repository navigation
genmedia-for-commerce: 3 variations and one video, one item per slot, mannequin skin rules - #2869
Conversation
…s, one item per slot, mannequin skin rules, review fixes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated House Rules review — 0 finding(s).
Also, on lines this PR does not change:
core/python/genmedia-for-commerce/genmedia4commerce/core/settings.py:184— [MINOR] env read carries a hardcoded default (second argument is a hardcoded default); 4 occurrence(s) across 1 file(s). Defaults belong in .env.example, not in the code (core/python/genmedia-for-commerce/genmedia4commerce/core/settings.py:184; and 3 more)
There was a problem hiding this comment.
Automated Security review — 0 finding(s).
core/python/genmedia-for-commerce/AGENTS.mdcore/python/genmedia-for-commerce/README.mdcore/python/genmedia-for-commerce/frontend/src/test/studioMock.tscore/python/genmedia-for-commerce/genmedia4commerce/eval/prompts/outfit_wearer.mdcore/python/genmedia-for-commerce/genmedia4commerce/requests.pycore/python/genmedia-for-commerce/genmedia4commerce/tools.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/tryon/classify.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/tryon/classify_prompt.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/video_tryon/pipeline.pycore/python/genmedia-for-commerce/tests/api/test_streaming_routes.pycore/python/genmedia-for-commerce/tests/core/test_settings.pycore/python/genmedia-for-commerce/tests/studio/test_agent_tools_order.pycore/python/genmedia-for-commerce/tests/studio/test_job_expected.pycore/python/genmedia-for-commerce/tests/studio/test_jobs_video.pycore/python/genmedia-for-commerce/tests/studio/test_studio.py- …and 20 more
Splitting the PR up would get them reviewed.
Round 1 · 0 of this PR's 25 automated comments used · this round is capped at 20 across all reviewers.
There was a problem hiding this comment.
Automated Maintainability review — 5 finding(s).
core/python/genmedia-for-commerce/AGENTS.mdcore/python/genmedia-for-commerce/README.mdcore/python/genmedia-for-commerce/frontend/src/test/studioMock.tscore/python/genmedia-for-commerce/genmedia4commerce/eval/prompts/outfit_wearer.mdcore/python/genmedia-for-commerce/genmedia4commerce/requests.pycore/python/genmedia-for-commerce/genmedia4commerce/tools.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/tryon/classify.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/tryon/classify_prompt.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/video_tryon/pipeline.pycore/python/genmedia-for-commerce/tests/api/test_streaming_routes.pycore/python/genmedia-for-commerce/tests/core/test_settings.pycore/python/genmedia-for-commerce/tests/studio/test_agent_tools_order.pycore/python/genmedia-for-commerce/tests/studio/test_job_expected.pycore/python/genmedia-for-commerce/tests/studio/test_jobs_video.pycore/python/genmedia-for-commerce/tests/studio/test_studio.py- …and 20 more
Splitting the PR up would get them reviewed.
Round 1 · 0 of this PR's 25 automated comments used · this round is capped at 20 across all reviewers.
| conflict (``hero.conflicts``) as settled; none without a ``rule``.""" | ||
| if rule is None: | ||
| return kept, [] | ||
| shown = {keys[id(p)] for p in kept if id(p) in keys} |
There was a problem hiding this comment.
[MINOR] Is there a reason we're keying by id(p) instead of p.id? Passing added_ids = {p.id for p in added} would avoid relying on object memory addresses.
| """The products the look adds: its items (``_look_items``) but, with | ||
| the rule "upload" (``resolved``), those in an uploaded product's place | ||
| (``conflicts``), each with its key ("g7") as id and no image yet.""" | ||
| out = set() |
There was a problem hiding this comment.
[MINOR] Perhaps rename out to excluded or conflicted? In conflicts() above out is the returned list, whereas here it represents items to omit.
| """(how it is worn, its layer, the rest) from its plan item.""" | ||
| item = plan[product.id] | ||
| how = [_clean(item.get(k)) for k in ("tuck", "closure")] | ||
| how[1] = _CLOSURE_SAY.get(how[1], how[1]) |
There was a problem hiding this comment.
[MINOR] Perhaps unpack into tuck, closure instead of indexing and mutating how[1]? Named variables would make the closure logic clearer.
| if rule == "look" and item.product not in shown: | ||
| continue # the look item was not cropped: the upload stays | ||
| aside |= {upload.id} if rule == "look" else set() | ||
| entry = {"kept": rule, "upload": upload.id} |
There was a problem hiding this comment.
[MINOR] combine into a single dict literal?
| out = [] | ||
| for item in _look_items(c): | ||
| rivals = [p for p in c.products if _in_place_of(item, p)] | ||
| out += [(item, rivals[0])] if rivals else [] |
There was a problem hiding this comment.
[MINOR] use out.append here instead
There was a problem hiding this comment.
Automated Correctness review — 1 finding(s).
core/python/genmedia-for-commerce/AGENTS.mdcore/python/genmedia-for-commerce/README.mdcore/python/genmedia-for-commerce/frontend/src/test/studioMock.tscore/python/genmedia-for-commerce/genmedia4commerce/eval/prompts/outfit_wearer.mdcore/python/genmedia-for-commerce/genmedia4commerce/requests.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/tryon/classify.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/tryon/classify_prompt.pycore/python/genmedia-for-commerce/genmedia4commerce/workflows/video_tryon/pipeline.pycore/python/genmedia-for-commerce/tests/api/test_streaming_routes.pycore/python/genmedia-for-commerce/tests/core/test_settings.pycore/python/genmedia-for-commerce/tests/eval/test_outfit.pycore/python/genmedia-for-commerce/tests/studio/test_agent_tools_order.pycore/python/genmedia-for-commerce/tests/studio/test_job_expected.pycore/python/genmedia-for-commerce/tests/studio/test_jobs_video.pycore/python/genmedia-for-commerce/tests/studio/test_studio.py- …and 21 more
Splitting the PR up would get them reviewed.
Round 1 · 0 of this PR's 25 automated comments used · this round is capped at 20 across all reviewers.
| ) | ||
| rules += [ | ||
| RULE_BODY.format(body=body), | ||
| RULE_BODY.format(body=body) + skin.OWN_SKIN.format(face=face or body), |
There was a problem hiding this comment.
[MAJOR] It seems RULE_BODY contains {images}, but only body is provided to format(). Will this raise a KeyError at runtime?
|
There is a problem with the Antigravity CLI Hygiene PR review. Please check the action logs for details. |
Summary
instead_of) no longer both get drawn.GENMEDIA_LOOK_CONFLICT(or the request'slook_conflict) picks which one stays:upload(default) orlook. A progress line says which was used.