fix: address 9 code-review findings#25
Merged
Merged
Conversation
…builder
- decompile/pptx_decompile: move pic counter list outside shape loop so
multiple pictures on a slide get distinct indices instead of all being 1
- io/image_preflight: guard _extract_dominant_colors against zero-pixel
images to prevent ZeroDivisionError
- verify/cache: write cache via temp-file + rename for atomic persistence;
a SIGINT mid-write no longer corrupts .verify_cache.json
- verify/autofix: replace __file__.parents path hack for layout lookup with
feinschliff.layout_discovery.find_layout, which works in installed mode
- verify/static: _flatten_content_keys now emits non-string leaf values
(int, float, bool) as str so numeric slots are visible to static checks
- deck/orchestrate: guard entry["content"] with `or {}` so a YAML slide
with content: ~ does not crash interpolate_nodes with ctx=None
- brand_discovery: require "kind" key to be present in $image_provider
dict before storing, preventing a KeyError in callers
- dsl/tokens: wrap bare json.loads in merge loop with a ValueError that
names the brand directory, replacing an opaque JSONDecodeError
- dsl/expander: use `is not None` for virtual_w/virtual_h fallback so an
explicit integer 0 is not silently replaced by the slot dimension
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Mike Mueller <mike@objektarium.de>
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
1 participant
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.
Summary
Nine bugs found during a recall-mode code review, fixed across both packages.
Critical (data loss / crash)
pptx_decompile.py—pic_idx_refcounter lambda was recreated inside the per-shape loop, giving every picture on a slidepic_idx=1and overwriting the same asset filename; counter list is now created once per slideimage_preflight.py—_extract_dominant_colorsdivided bytotal_pixelswithout guarding against zero; 0×N images now return early with[]deck/orchestrate.py—entry["content"]was passed asctxtointerpolate_nodeswithout a None-check; a YAML slide withcontent: ~crashed the buildHigh (feature broken / silent wrong result)
verify/cache.py—save()wrote directly viawrite_text, leaving a truncated.verify_cache.jsonon SIGINT; now uses temp-file +Path.replacefor atomic writeverify/autofix.py—_find_smaller_layout/_find_larger_layoutresolved core layout paths via__file__.parents[2].parent, which is wrong in any pip-installed deployment; now usesfeinschliff.layout_discovery.find_layoutverify/static.py—_flatten_content_keysonly emittedstrleaves;int/float/boolslot values were silently dropped, causing false EMPTY_PLACEHOLDER warnings for numeric contentLow (edge cases / poor error messages)
brand_discovery.py—$image_providerdict was stored without checking for thekindkey; callers docfg["kind"]directly, so a misconfigured brand would crash with aKeyErrorinstead of a clear messagedsl/tokens.py— barejson.loads()in the brand-inheritance merge loop had no error handling; now wraps with aValueErrornaming the brand directorydsl/expander.py—virtual_w/virtual_husedor w/or hfallback; an explicit integer0was silently replaced by the slot dimensionTest plan
uv run pytest feinschliff/tests/ feinschliff-builder/tests/ -x -q— 727 pass, 1 pre-existing skip (npxnot installed), no regressionsfeinschliff-builder verifyon a deck, send SIGINT mid-run, restart — cache should be intact🤖 Generated with Claude Code