Skip to content

fix(vis): custom __repr__ overlays lazy state on field section (closes mat#221)#232

Merged
gerchowl merged 2 commits into
mainfrom
fix/vis-repr-observability
May 8, 2026
Merged

fix(vis): custom __repr__ overlays lazy state on field section (closes mat#221)#232
gerchowl merged 2 commits into
mainfrom
fix/vis-repr-observability

Conversation

@gerchowl
Copy link
Copy Markdown
Contributor

@gerchowl gerchowl commented May 8, 2026

Summary

Closes mat#221 — Bernhard's mat-vis#311 'Inconsistent outputs' sub-bullet. After v.textures triggers a fetch, repr(v) still showed every PBR field as None — no signal anything happened, even though the catalog data was loaded.

Pythonic decision: two namespaces, two purposes

The dataclass auto-repr was technically truthful (those fields are the override channel — user hadn't set them) but visually misleading. Two ways to fix:

Approach Trade-off
Populate dataclass fields after fetch Forces a magic distinction between "user-set" and "catalog-set" — needs an _overridden: set[str] to disambiguate, breaks replace / equality / pickle semantics, conflates inputs with outputs
Custom __repr__ overlaying lazy state Field semantics unchanged; scalars= and available_textures= surface the resolved/cached view. Two clearly separate sections, no provenance ambiguity

We took the second. Same separation already pioneered by Vis.scalars (the public sparse view from the dispatch refactor): dataclass fields = caller overrides; property accessors = computed/resolved.

Output shape

Pre-fetch:

Vis(source='gpuopen', material_id='Aluminum Brushed', tier='1k',
    finishes={}, roughness=None, metallic=None, ..., fetched=False)

Post-fetch (catalog-backed):

Vis(source='gpuopen', material_id='Aluminum Brushed', tier='1k',
    finishes={}, roughness=None, metallic=None, ..., fetched=True,
    scalars={'metalness': 1.0, 'roughness': 0.4, 'color_hex': '#cccccc', 'ior': 1.5},
    available_textures=['color', 'normal', 'roughness'])

Post-fetch with caller override (v.metallic = 0.7):

Vis(source='gpuopen', material_id='Aluminum Brushed', tier='1k',
    finishes={}, roughness=None, metallic=0.7, ..., fetched=True,
    scalars={'metalness': 0.7, ...},   ← override won; visible in BOTH places
    available_textures=['color', 'normal', 'roughness'])

Implementation notes

  • Touches self.scalars (catalog dict lookup) only when _fetched=True — unfetched repr stays cheap, no index IO triggered by print(v) or debugger inspection.
  • Honors field(repr=False) on private cache slots (_textures, _fetched, _finish) — same fields excluded as the auto-repr.
  • ~50 LOC total: 1 method on Vis, drops xfail decorator on 3 tests.

Test plan

  • All 3 TestIssue221ReprObservability xfails flip to passing
  • test_repr_does_not_leak_private_path (the existing repr contract test) still passes — class name + module unchanged
  • No regression in 968-test full suite (vs 968 on main pre-fix; xfail count drops 11 → 8)
  • Manual visual check across 4 cases (empty / overrides-only / pre-fetch / post-fetch with overrides) — all read clearly

Out of scope

Bernhard's remaining mat-vis#280-rooted error annotation gaps in test_error_messages.py (5 xfails) — separate concern, separate PR.

Refs mat#221, Bernhard's mat-vis#311

gerchowl added 2 commits May 8, 2026 11:56
…s mat#221)

Bernhard's mat-vis#311 'Inconsistent outputs' sub-bullet: after
``v.textures`` triggers a fetch, ``repr(v)`` still shows every PBR
field as ``None`` — no signal anything happened. The dataclass
auto-repr is technically truthful (those fields are the *override*
channel; user hasn't overridden them), but it leaves Bernhard
debugging from a half-truth.

Pythonic fix: keep two namespaces, two purposes — don't conflate
overrides with resolved state. The dataclass field section keeps its
override-channel semantics; ``__repr__`` appends a lazy-state suffix
so both perspectives are legible at a glance.

Pre-fetch::

    Vis(source='gpuopen', material_id='Aluminum Brushed', tier='1k',
        finishes={}, roughness=None, metallic=None, ..., fetched=False)

Post-fetch (catalog-backed)::

    Vis(source='gpuopen', material_id='Aluminum Brushed', tier='1k',
        finishes={}, roughness=None, metallic=None, ..., fetched=True,
        scalars={'metalness': 1.0, 'roughness': 0.4, 'color_hex': '#cccccc', 'ior': 1.5},
        available_textures=['color', 'normal', 'roughness'])

Post-fetch with caller override (``v.metallic = 0.7``)::

    Vis(source='gpuopen', material_id='Aluminum Brushed', tier='1k',
        finishes={}, roughness=None, metallic=0.7, ..., fetched=True,
        scalars={'metalness': 0.7, 'roughness': 0.4, ...},  ← override won
        available_textures=['color', 'normal', 'roughness'])

Field section reflects what *the user* set (None = no override).
``scalars=`` reflects what will actually render (catalog merged with
overrides — same view as ``Vis.scalars``). Both visible without
inverting either's meaning.

Implementation notes:

- Touches ``self.scalars`` only when ``_fetched=True`` so an
  unfetched repr doesn't trip catalog index loads.
- Honors ``field(repr=False)`` on private cache slots (``_textures``,
  ``_fetched``, ``_finish``).
- 3 strict-xfail tests in ``TestIssue221ReprObservability`` flip to
  passing — xfail markers dropped.
@gerchowl gerchowl force-pushed the fix/vis-repr-observability branch from 1fd47d4 to da39df8 Compare May 8, 2026 10:02
@gerchowl gerchowl merged commit bc6536d into main May 8, 2026
21 checks passed
@vig-os-release-app vig-os-release-app Bot mentioned this pull request May 8, 2026
7 tasks
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.

1 participant