fix: make _lazy_construct respect discriminator fields

[RapidPoseidon]

Summary

Fixes the ValueError: Multiple matches found when deserializing the JSON string into IArtifactModel with oneOf schemas: ... regression introduced by #537 (lazy validation), reported on the display_progress_bar flow (GetPipelineByIdResult.from_dict -> IArtifactModel.from_dict -> IArtifactModel.from_json).

Root cause

oneOf/anyOf disambiguation loops in the generated code look like:

for variant in variants:
    try:
        instance.actual_instance = Variant.from_json(json_str)
        match += 1
    except (ValidationError, ValueError):
        error_messages.append(...)

This relies on non-matching variants raising ValidationError. After #537, Variant.from_json -> from_dict catches ValidationError and falls back to _lazy_construct, so every variant "succeeds", match > 1, and the branch raises "Multiple matches found".

Fix

Distinguish "this variant is wrong" from "a field drifted" inside _lazy_construct itself:

• A strict field is any field with a custom Pydantic field_validator. The OpenAPI generator already emits one on every discriminator field (_t) that enforces the enum value ("FileArtifactModel" etc.).
• If any validation error targets a strict field -> re-raise the original ValidationError. The model choice is wrong, not drifting.
• Otherwise -> lazy-construct as before, defer errors to field-access time.

No template or generated-file changes — the discriminator handling is fully in the base class, auto-detected via __pydantic_decorators__.field_validators.

Why this is better than bypassing lazy in the oneOf loop

An earlier version of this PR patched the oneOf/anyOf templates to use model_validate_json (strict) in the disambiguation loop. That worked but lost lazy protection for fields inside the picked variant — drift on an unused field of the chosen oneOf variant would crash the deserialization entirely.

The strict-discriminator approach preserves the lazy benefit everywhere: once _t picks the right variant, any non-discriminator drift inside that variant is still absorbed.

Verified scenarios

1. Direct variant, valid payload — works
2. Direct variant, drift on unused field — lazy fires, TypeError on bad field access
3. Direct variant, wrong _t — re-raises ValidationError
4. IArtifactModel.from_dict via oneOf (the reported bug) — picks correct variant
5. oneOf + drift on non-strict field inside matching variant — variant picked, lazy on bad field (the critical case)
6. oneOf on gibberish — raises (no silent junk)

Files touched

• src/rapidata/api_client/lazy_model.py — one method added (_strict_field_names), one early-return added in _lazy_construct.

No template changes. No generated-file changes.

[claude]

Code Review: PR #539 — fix oneOf/anyOf 'Multiple matches found' regression

Overview

This PR correctly fixes a regression from #537 where LazyValidatedModel.from_json's fallback to _lazy_construct (on ValidationError) caused every oneOf/anyOf variant to appear as a valid match, producing spurious "Multiple matches found" errors. The fix uses model_validate_json (strict Pydantic) in the disambiguation loop, restoring the pre-#537 invariant that non-matching variants raise.

---

Correctness — Approved with caveats

Root cause analysis is accurate. The disambiguation loop depended on from_json raising ValidationError to skip non-matching variants. After #537, from_json swallows ValidationError and falls back to _lazy_construct, so every variant "succeeded". Using model_validate_json (which does not fall back) restores the exception-based filtering.

The discriminator path is intentionally left as from_json (model_oneof.mustache line 126). That path already knows the variant; it's not disambiguating. Keeping lazy protection there is correct.

anyOf vs. oneOf semantics are handled correctly. anyOf exits on the first match (early return), oneOf counts all matches. Both benefit from the stricter probe.

---

Issues and Risks

1. Lazy protection (#537) is partially surrendered for matched variants in a oneOf/anyOf context (important)

model_validate_json is strict end-to-end — it validates every field in the winning variant. If the backend adds a required field or changes a field type in a oneOf variant, the strict probe will fail for that variant even when the payload is intended for it. The disambiguator will report "No match found" instead of falling back gracefully. The PR's claim that "lazy protection still fires for variants used directly via from_dict" is true — but only when the concrete variant model is called directly, not when it is reached through a oneOf/anyOf wrapper. This narrows the schema-drift resilience that #537 was designed to provide; it's worth documenting explicitly.

2. No regression test added

The test plan lists manual checks but no automated test is included. For a fix to a serialization regression, a unit test would be valuable:

• Input: JSON that is valid for exactly one oneOf variant
• Assert: deserialization picks the correct variant without raising
• Input: gibberish JSON
• Assert: "No match found" is raised
• Input: valid concrete variant JSON deserialized directly via from_dict
• Assert: lazy protection still works (field type mismatch doesn't raise immediately)

Given that CI isn't passing yet, and the fix touches 195 generated call sites, a targeted test would give confidence that the fix holds.

3. 195 hand-patched call sites across 37 files — maintenance risk

The PR correctly notes that auto-regen is blocked by upstream schema-name conflicts. The risk is that when regen is unblocked, these changes must come from the updated templates — not be dropped. Consider adding a comment to the generate-schema.sh or a CI check as a reminder that model_validate_json must be used in oneOf/anyOf disambiguation. The template fix alone ensures future regenerations are correct, but the window while regen is blocked is a risk.

4. Duplicate imports in generated files

In i_artifact_model.py (and likely other generated files), there are duplicate imports:

# line 18
from pydantic import BaseModel, ConfigDict, Field, StrictStr, ValidationError, field_validator
# line 25 (duplicate)
from pydantic import StrictStr, Field

This is a generated-code artifact, but it's worth cleaning up in the template to avoid linter warnings.

5. Verbose comments in templates (minor)

The added comments span 5–6 lines explaining the from_json vs. model_validate_json distinction. Per project conventions, comments should be short. The key point fits in one line:

# strict probe: lazy fallback in from_json would make every variant succeed
instance.actual_instance = {{{dataType}}}.model_validate_json(json_str)

---

Summary

The fix is technically correct and unblocks the display_progress_bar flow. The template update ensures the fix survives future regen. The main thing to be aware of going forward is that oneOf/anyOf matched variants no longer receive #537's lazy protection — schema drift on a field of the selected variant will now raise at deserialization time rather than being deferred to field-access time.

Recommend:

• Add a regression test for the oneOf disambiguation path
• Shorten template comments to a single line
• Fix duplicate import lines in the mustache templates
• Document the reduced scope of lazy protection in the discriminated-union case (e.g., in a # KNOWN LIMITATION comment in lazy_model.py or the PR description)

[claude]

Code Review

Overview

This PR fixes a regression from #537 where all oneOf/anyOf variants appeared to match during deserialization, causing a "Multiple matches found" error. The root cause: lazy validation in LazyValidatedModel._lazy_construct was swallowing ValidationError for non-matching variants, making every variant "succeed".

The fix is elegant — instead of patching 195 call sites (the original plan described), it teaches _lazy_construct to re-raise errors that touch discriminator fields (fields with field_validator decorators), while still allowing lazy fallback for ordinary schema-drift failures.

---

What the fix actually does (vs. what the PR description says)

The PR description's "What changed" section lists three items:

• openapi/templates/model_oneof.mustache — template fix
• openapi/templates/model_anyof.mustache — template fix
• src/rapidata/api_client/models/*.py — 195 hand-patched call sites

None of these appear in the diff. The actual diff is 39 lines entirely in lazy_model.py. The templates still use {{{dataType}}}.from_json(json_str) in the disambiguation loop — which is fine because the lazy_model.py fix makes from_json raise correctly again. But the PR description should be updated to avoid confusion during future triage.

---

Code quality

Strengths:

• The mechanism is well-chosen: field_validator-decorated fields are exactly the generated discriminator fields (_t), so detecting them via __pydantic_decorators__ is a clean heuristic
• Ordering of operations is correct and efficient — strict-field check happens before building construct_kwargs (no wasted work on the re-raise path)
• Good inline documentation explaining the invariant

Issues:

1. Relies on private Pydantic internals — silent failure risk

decorators = getattr(cls, "__pydantic_decorators__", None)
if decorators is None:
    return set()  # silently falls back — no strict fields detected

__pydantic_decorators__ is not a public Pydantic API. If its structure changes in a future Pydantic release, _strict_field_names() will silently return set(), re-enabling the original regression with no error or warning. Consider adding a smoke-test assertion (or at least a comment) that validates this assumption against the installed Pydantic version.

2. _strict_field_names() is not cached

The method is called on every _lazy_construct invocation via reflection. This is the error path so performance isn't critical, but the result is fully static — it could be @functools.lru_cache(None) or cached at class definition time (e.g., via __init_subclass__) without any loss of correctness.

3. Overly broad "strict" definition

Any field with a field_validator is treated as strict. Today, in the generated code, that's only discriminator fields (_t). But:

• If a future schema revision adds format-validation decorators on non-discriminator fields, those would also become "strict" and suppress lazy construction — silently breaking the drift-tolerance this system provides
• A comment explaining this assumption (and that it holds because the generator only emits field_validator for discriminator checks) would prevent future maintainers from accidentally adding validators

4. Return type annotation

def _strict_field_names(cls) -> set:  # should be -> set[str]

Minor, but consistent with the rest of the file's type annotations. Since from __future__ import annotations is already present, set[str] works on Python 3.9+.

---

Correctness

The fix correctly covers the described failure path:
IArtifactModel.from_json → IArtifactModelFileArtifactModel.from_json → from_dict → model_validate → ValidationError on _t → _lazy_construct now re-raises → outer try/except catches it → next variant tried.

One subtle interaction worth verifying: IArtifactModel itself has @field_validator('actual_instance'), so _strict_field_names() on IArtifactModel returns {'actual_instance'}. This is correct behavior (you don't want lazy construction of the container with an invalid actual_instance), but it's an implicit side-effect worth calling out.

---

Test coverage

• No tests added for _strict_field_names()
• No regression test for the described failure path (GetPipelineByIdResult.from_dict → IArtifactModel.from_dict)
• The test plan in the description is entirely manual ("reproduced locally")

Given this is fixing a regression that previously went undetected until production, a regression test would be valuable here.

---

Summary

The core fix is sound and well-targeted. Main asks before merging:

1. Update the PR description to reflect that the fix is only in lazy_model.py (not templates or model files)
2. Add a comment in _strict_field_names() noting the assumption that field_validator == discriminator field in generated code, and the risk if Pydantic's __pydantic_decorators__ structure changes
3. Consider a regression test for the from_json → oneOf disambiguation path
4. Minor: tighten the return type to set[str]

[claude]

Code Review

Overview

This PR fixes a regression from #537 where lazy validation caused all oneOf/anyOf variants to "succeed" during disambiguation (since ValidationError was caught and swallowed), resulting in ValueError: Multiple matches found. The fix is focused and correct: re-raise ValidationError only when the _t discriminator field fails, preserving lazy behavior for every other field.

---

Code Quality

_strict_field_names method

• Missing return type annotation. Per the project's conventions and the existing use of from __future__ import annotations, this should be -> set[str] (or frozenset[str] since the result is never mutated):

  @classmethod
  def _strict_field_names(cls) -> set[str]:

• The method is called on every _lazy_construct invocation and iterates model_fields each time. Since model fields are static class attributes that never change at runtime, this is a good candidate for @functools.lru_cache or a functools.cached_property equivalent. That said, _lazy_construct is only reached on the error path, so this is a minor concern, not a blocker.

_lazy_construct restructuring

• Moving construct_kwargs building after the strict-field check is correct — no point populating kwargs if we're about to re-raise. Good ordering.
• raise error correctly re-raises the original ValidationError with its traceback, which is the right pattern here.

---

Potential Issues

1. Hardcoded "_t" alias: The discriminator alias "_t" is assumed to be the only structural discriminator across all generated models. This is brittle if the OpenAPI generator ever uses a different discriminator alias. Consider a named constant or a brief comment that ties this to the generator convention, making the coupling explicit:

   _DISCRIMINATOR_ALIAS = "_t"  # emitted by the OpenAPI generator for all oneOf variants

2. PR description vs. implementation discrepancy: The PR description says the discriminator is "auto-detected via __pydantic_decorators__.field_validators", but the actual implementation detects it via field_info.alias == "_t". These are two different mechanisms — the description is misleading and should be updated to match the implementation.

3. Edge case — _t field appears in loc[0] as alias: The alias_to_field mapping in _lazy_construct maps both alias → python_name and python_name → python_name. Pydantic v2 error.errors() returns loc using either the Python field name or the alias depending on validation mode. Since populate_by_name=True is set, it is worth verifying that loc[0] in the _t discriminator error is always the alias (not the Python name like type) before the intersection check. If it's the Python name, _strict_field_names (which returns the Python name via field_name) will still match correctly — so the logic is likely fine, but a comment or test making this explicit would be valuable.

---

Test Coverage

The PR description lists 6 "Verified scenarios" that read like manual test cases. None of them appear as automated tests in the diff. Given that this is a regression fix, adding a small unit test covering at minimum scenarios 3 and 4 (wrong _t re-raises; correct variant picked via oneOf) would prevent this from regressing again and would serve as living documentation of the expected behavior.

---

Summary

Correctness	✅ Fix logic is sound and targets the right failure mode
Code style	⚠️ Missing return type annotation on _strict_field_names
Robustness	⚠️ Hardcoded "_t" alias and discrepancy with PR description
Tests	⚠️ No automated tests added for a regression fix
Security	✅ No concerns

The core fix is correct and well-reasoned. The main asks before merge are:

1. Add the -> set[str] return type annotation.
2. Update the PR description (or a code comment) to accurately reflect that the detection uses field_info.alias, not __pydantic_decorators__.
3. Consider a lightweight unit test for the oneOf disambiguation scenarios.