refactor(models)!: relax the schema-v2 mirrors from extra="forbid" to extra="ignore"

cldk/models/java/models.py

@@ -32,7 +32,11 @@
 ``refs``, the ``is_*`` type predicates over ``kind`` and the owner chain. What the wire does not
 carry (CRUD) is an empty list, and the facade raises for it (J-4).
 
-``extra="forbid"`` is intentional: drift between the analyzer's JSON and these models fails loudly.
+``extra="ignore"`` since #386: an additive analyzer release is consumable without an SDK edit. What
+that gives up is the drift detector — an unmodelled field is dropped silently, so a field the SDK does
+not declare is unreachable rather than an error. The projections in ``projections.py`` keep
+``extra="forbid"``, because they are constructed by this SDK and never validated from the wire, so
+there strictness guards our own typos rather than the analyzer's additions.
 """
 
 from __future__ import annotations
@@ -46,7 +50,26 @@
 
 
 class _Base(BaseModel):
-    model_config = ConfigDict(extra="forbid")
+    #: ``ignore``, not ``forbid`` (#386). An additive analyzer release used to fail the whole payload
+    #: rather than the one field it added: codeanalyzer-java 3.1.2's ``var`` on ``param_in``/
+    #: ``param_out`` produced 2515 validation errors on daytrader8 until ``JParamEdge`` declared it,
+    #: for a change that was backward compatible by construction. Uptake should not require an SDK
+    #: edit before anything parses.
+    #:
+    #: What this gives up is the drift detector. ``forbid`` was what surfaced that ``var`` within
+    #: seconds of the pin bump; under ``ignore`` the same addition is absorbed silently and the first
+    #: symptom is a wrong answer from something reading a field the SDK never learned. The intended
+    #: replacement is a comparison against each analyzer's published schema in ``codeanalyzer-schema``
+    #: — reporting what is unmodelled instead of refusing to parse — which is tracked separately.
+    #:
+    #: ``ignore`` rather than ``allow`` on purpose: ``allow`` keeps unknown fields in ``model_extra``
+    #: and so widens ``model_dump_json()`` with whatever the analyzer emitted, and several tests
+    #: assert properties *of* dumps (E6's ``"can://" not in ...model_dump_json()``). A field the SDK
+    #: does not model must not be able to change what a dump contains.
+    #:
+    #: A declared field is therefore the only way a value is reachable. Relaxing this does not make
+    #: the explicit ones redundant — it makes them load-bearing.
+    model_config = ConfigDict(extra="ignore")
 
 
 # ----------------------------------------------------------------------------------------------
@@ -653,7 +676,9 @@ class JCompilationUnit(_Node):
     :class:`JImport` records, exposed as :attr:`import_declarations`; the 1.x ``imports`` (a list of
     paths) is the property of that name."""
 
-    model_config = ConfigDict(extra="forbid", validate_by_name=True, validate_by_alias=True, serialize_by_alias=True)
+    #: Overrides ``_Base`` entirely, so it needs its own ``ignore`` (#386) — a subclass
+    #: ``model_config`` replaces rather than merges, and this one is here for the alias settings.
+    model_config = ConfigDict(extra="ignore", validate_by_name=True, validate_by_alias=True, serialize_by_alias=True)
 
     kind: Literal["module"] = "module"
     span: JSpan

cldk/models/typescript/models.py

@@ -31,7 +31,8 @@
 ``classes``/``interfaces``/… maps over the unified ``types``). What the wire no longer carries
 (``path``, ``accessed_symbols``, ``local_variables``, ``code_start_line``) is gone, not faked.
 
-``extra="forbid"`` is intentional: drift between the analyzer's JSON and these models fails
+``extra="ignore"`` since #386: an additive analyzer release is consumable without an SDK edit, at the
+cost of the drift detector — an unmodelled field is dropped silently rather than failing
 loudly. The fields the next analyzer release (1.3.0) is known to add are already declared
 ``Optional`` so that pin bump changes no model.
 """
@@ -45,7 +46,26 @@
 
 
 class _Base(BaseModel):
-    model_config = ConfigDict(extra="forbid")
+    #: ``ignore``, not ``forbid`` (#386). An additive analyzer release used to fail the whole payload
+    #: rather than the one field it added: codeanalyzer-java 3.1.2's ``var`` on ``param_in``/
+    #: ``param_out`` produced 2515 validation errors on daytrader8 until ``JParamEdge`` declared it,
+    #: for a change that was backward compatible by construction. Uptake should not require an SDK
+    #: edit before anything parses.
+    #:
+    #: What this gives up is the drift detector. ``forbid`` was what surfaced that ``var`` within
+    #: seconds of the pin bump; under ``ignore`` the same addition is absorbed silently and the first
+    #: symptom is a wrong answer from something reading a field the SDK never learned. The intended
+    #: replacement is a comparison against each analyzer's published schema in ``codeanalyzer-schema``
+    #: — reporting what is unmodelled instead of refusing to parse — which is tracked separately.
+    #:
+    #: ``ignore`` rather than ``allow`` on purpose: ``allow`` keeps unknown fields in ``model_extra``
+    #: and so widens ``model_dump_json()`` with whatever the analyzer emitted, and several tests
+    #: assert properties *of* dumps (E6's ``"can://" not in ...model_dump_json()``). A field the SDK
+    #: does not model must not be able to change what a dump contains.
+    #:
+    #: A declared field is therefore the only way a value is reachable. Relaxing this does not make
+    #: the explicit ones redundant — it makes them load-bearing.
+    model_config = ConfigDict(extra="ignore")
 
 
 # ----------------------------------------------------------------------------------------------
@@ -417,7 +437,7 @@ class _Type(_Spanned):
     is_ambient: bool = False
     # 1.3.0 additive, on *every* type kind (TS-8): the analyzer stamps the entrypoint tier on
     # whatever declaration a ruleset matched, so an interface, enum, type alias or namespace can
-    # carry it too. Declaring it only on TSClass would fail ``extra="forbid"`` on those four the
+    # carry it too. Declaring it only on TSClass would have failed the old ``extra="forbid"`` on those four the
     # day the pin moves -- exactly the breakage TS-8 exists to prevent.
     entrypoints: Optional[List[TSEntrypoint]] = None
     is_entrypoint: Optional[bool] = None

tests/models/java/test_java_models.py

@@ -80,19 +80,48 @@ def test_round_trip_is_byte_equal(fixture_name: str, request):
     assert _sorted(dumped) == _sorted(json.loads(raw))
 
 
-def test_unknown_top_level_key_is_rejected(analysis_json_a4: str):
+def test_an_unknown_top_level_key_is_ignored(analysis_json_a4: str):
+    """#386: the mirrors are ``extra="ignore"``, so an undeclared key is dropped, not rejected.
+
+    ``repository`` is a real field on codeanalyzer-python's application and absent from Java's, so it
+    stands in for the shape this policy exists to absorb: a sibling analyzer's field arriving in a
+    later Java release. It used to fail the whole payload; now it parses and the value is gone.
+    """
     raw = json.loads(analysis_json_a4)
     raw["repository"] = "x"
-    with pytest.raises(ValidationError):
-        JAnalysis.model_validate(raw)
+
+    a = JAnalysis.model_validate(raw)
+    assert not hasattr(a, "repository")
+    assert "repository" not in a.model_dump()
+    assert a.model_extra in (None, {}), "extra=ignore must not retain it; extra=allow would"
 
 
-def test_unknown_nested_key_is_rejected(analysis_json_a4: str):
+def test_an_unknown_nested_key_is_ignored(analysis_json_a4: str):
+    """The same policy one level down, where the old behaviour was most expensive.
+
+    ``file_path`` is a **retired 1.x** key on the compilation unit. Rejecting it meant a stale or
+    misspelled wire key failed the entire analysis; ignoring it means the key is unreachable and
+    nothing says so. That is the trade #386 accepted, and the assertion here is what keeps it
+    explicit rather than folklore.
+
+    ``JCompilationUnit`` is worth naming: it overrides ``model_config`` wholesale for its alias
+    settings, so ``_Base``'s policy does not reach it and it carries its own ``ignore``. If this test
+    ever fails while the top-level one passes, that override has drifted back to ``forbid``.
+    """
     raw = json.loads(analysis_json_a4)
-    unit = next(iter(raw["application"]["symbol_table"].values()))
-    unit["file_path"] = "x"
-    with pytest.raises(ValidationError):
-        JAnalysis.model_validate(raw)
+    key = next(iter(raw["application"]["symbol_table"]))
+    raw["application"]["symbol_table"][key]["file_path"] = "x"
+
+    a = JAnalysis.model_validate(raw)
+    unit = a.application.symbol_table[key]
+
+    # `file_path` is a property over a PrivateAttr that `JApplication` stamps from the symbol-table
+    # key (models.py:691,714) -- not a wire field -- so the test is not that the attribute vanishes
+    # but that the injected wire value never reaches it.
+    assert "file_path" not in type(unit).model_fields, "file_path is not a wire field"
+    assert unit.file_path == key, "the property still derives from the symbol-table key"
+    assert unit.file_path != "x", "the ignored wire value must not have taken effect"
+    assert unit.model_extra in (None, {}), "extra=ignore must not retain it; extra=allow would"
 
 
 def test_v1_shaped_payload_is_rejected():

tests/models/typescript/test_ts_v2_models.py

@@ -224,11 +224,32 @@ def test_1x_shaped_document_is_rejected():
         TSApplication.model_validate(v1)
 
 
-def test_unknown_field_is_rejected():
+def test_an_unknown_field_is_ignored_and_a_v1_payload_is_still_refused():
+    """#386 relaxed the mirrors to ``extra="ignore"``, and this test carries both halves of what that
+    changed and did not change.
+
+    ``file_path`` is a **retired 1.x** key. It used to make the payload fail; now it is dropped, which
+    is the cost of the relaxation stated plainly — a stale or misspelled wire key reads as a missing
+    feature rather than an error.
+
+    What relaxation does **not** touch is required-field validation, and that is the guard actually
+    doing the version work: the test above feeds a whole v1 payload and it is still refused, because
+    it *lacks* fields v2 requires rather than carrying extra ones. So "a 1.x analysis.json is refused,
+    not parsed" survives #386 intact. Asserting that here keeps the two mechanisms from being
+    confused for each other the next time someone weighs this policy.
+    """
     raw = json.loads((FIXTURES / "a1" / "analysis.json").read_text(encoding="utf-8"))
-    raw["application"]["symbol_table"][next(iter(raw["application"]["symbol_table"]))]["file_path"] = "x"
+    key = next(iter(raw["application"]["symbol_table"]))
+    raw["application"]["symbol_table"][key]["file_path"] = "x"
+
+    a = TSAnalysis.model_validate(raw)
+    module = a.application.symbol_table[key]
+    assert not hasattr(module, "file_path")
+    assert "file_path" not in module.model_dump()
+    assert module.model_extra in (None, {})
+
     with pytest.raises(ValidationError):
-        TSAnalysis.model_validate(raw)
+        TSAnalysis.model_validate({"schema_version": "2.0.0", "language": "typescript"})
 
 
 def test_unknown_type_kind_is_rejected():
@@ -240,9 +261,11 @@ def test_unknown_type_kind_is_rejected():
 
 
 def test_every_type_kind_accepts_the_1_3_0_entrypoint_fields():
-    # TS-8: both fields are declared on the shared ``_Type`` base, so a 1.3.0 payload that stamps
-    # them on any of the five kinds validates under ``extra="forbid"`` before the pin moves --
-    # declaring them on ``TSClass`` alone would fail validation on the other four.
+    # TS-8: both fields are declared on the shared ``_Type`` base, so a payload that stamps them on
+    # any of the five kinds is read on all five. Under the old ``extra="forbid"`` this was enforced by
+    # validation failing on the other four; since #386 relaxed the mirrors to ``ignore``, a field
+    # declared on ``TSClass`` alone would be *dropped* on the rest rather than raising -- so the
+    # positive assertions below are now what catches it, and they must stay positive for that reason.
     span = {"start": (1, 1), "end": (2, 1), "bytes": (0, 4)}
     entrypoint = {"framework": "express", "route": "/x", "http_methods": ["GET"]}
     for cls, kind in ((TSClass, "class"), (TSInterface, "interface"), (TSEnum, "enum"), (TSTypeAlias, "type_alias"), (TSNamespace, "namespace")):
@@ -253,19 +276,24 @@ def test_every_type_kind_accepts_the_1_3_0_entrypoint_fields():
         assert cls.model_validate({k: v for k, v in raw.items() if k not in ("is_entrypoint", "entrypoints")}).is_entrypoint is None, kind
 
 
-def test_the_pinned_generation_parses_with_nothing_widened():
+def test_the_pinned_generation_carries_the_fields_this_leg_declared():
     """The 1.3.0 bump moved no model (leg 2.5b, Task 0).
 
-    Every fixture is 1.3.0 output and every model is ``extra="forbid"``, so a field 1.3.0 added
-    that 2.5a had not pre-declared would be a ``ValidationError`` here, not a silent pass. What
+    **What this test proves changed with #386.** It used to rest on ``extra="forbid"``: every model
+    rejected extras, so merely parsing a fixture proved no field had appeared that the models had not
+    pre-declared. Since the mirrors are ``extra="ignore"``, parsing proves nothing of the kind — an
+    undeclared field is dropped in silence. So the assertions below carry the whole test now, and they
+    are positive on purpose: each one names a field and demands its value, which is the only remaining
+    way to notice it went missing. Adding a field to the fixtures without declaring it here will pass
+    silently, and that is the accepted cost of #386 rather than an oversight in this test. What
     1.3.0 added over 1.2.0 in this corpus: ``application.entrypoint_report`` (**required** on the
     application in 1.3.0's ``schema.ts``, kept optional here because the graph-backed application
     view carries the report as a JSON string on the anchor instead), ``is_entrypoint`` /
     ``entrypoints`` on classes and callables, ``parameters[].id`` and body-node ``id``s, and the
     L4 port lattice wired into the statement DDG.
     """
     for level in (1, 2, 3, 4):
-        a = _load(level)  # extra="forbid": this line is the assertion
+        a = _load(level)  # since #386 this line only parses; the assertions below are the test
         report = a.application.entrypoint_report
         assert report is not None, f"a{level} carries no entrypoint_report"
         assert report.rulesets == ["shipped"]
@@ -332,3 +360,52 @@ def test_span_bytes_are_utf8_offsets_and_code_decodes_them():
             drifted += module.source[start:end] != expected
     assert checked, "no non-ASCII module in the fixture; this test proves nothing as written"
     assert drifted, "every span in the non-ASCII modules starts before the first multi-byte character; the test cannot fail"
+
+
+# ----------------------------------------------------------------------------------------------
+# #386: the mirrors ignore what they do not declare. The projections do not.
+# ----------------------------------------------------------------------------------------------
+
+
+def test_the_schema_mirrors_ignore_unknown_fields_and_drop_them():
+    """The models accept a field they do not declare, and — the half worth asserting — **discard** it.
+
+    This is the policy #386 chose over ``extra="forbid"``, and the trade is deliberate: forbidding
+    made an additive analyzer release fail the whole payload rather than the one field it added
+    (codeanalyzer-java 3.1.2's ``var`` cost 2515 validation errors on daytrader8 before
+    ``JParamEdge`` declared it). What it gives up is that a new field no longer announces itself, so
+    a caller reaching for analyzer data the SDK does not model finds nothing rather than an error.
+    That is written down here rather than left to be discovered.
+
+    ``ignore`` and not ``allow``: the value must not survive into ``model_extra`` or a dump, because
+    several tests assert properties *of* dumps (E6's ``"can://" not in ...model_dump_json()``), and a
+    field the SDK does not model must not be able to change what a dump contains.
+    """
+    from cldk.models.java.models import JParamEdge
+    from cldk.models.typescript.models import TSParamEdge
+
+    for model in (TSParamEdge, JParamEdge):
+        assert model.model_config.get("extra") == "ignore", model.__name__
+        e = model.model_validate({"src": "a", "dst": "b", "var": "v", "field_from_a_future_release": 1})
+        assert e.var == "v", "a declared field is still read"
+        assert not hasattr(e, "field_from_a_future_release")
+        assert "field_from_a_future_release" not in e.model_dump()
+        assert e.model_extra in (None, {}), "extra=ignore must not retain it; extra=allow would"
+
+
+def test_the_sdk_authored_projections_still_forbid_extras():
+    """Scope of #386: the mirrors relaxed, the projections did not, and the difference is not arbitrary.
+
+    A projection is constructed by this SDK from graph rows and is never validated from an
+    ``analysis.json`` — nothing in the codebase calls ``model_validate`` on one. So strictness there
+    catches *our* typo'd keyword argument, not the analyzer's additions, and relaxing it would give up
+    a real guard for no uptake benefit.
+    """
+    from pydantic import ValidationError
+
+    from cldk.models.java.projections import JCallableOverview, JClassOverview
+
+    for model in (JCallableOverview, JClassOverview):
+        assert model.model_config.get("extra") == "forbid", model.__name__
+    with pytest.raises(ValidationError):
+        JClassOverview(qualified_name="a.B", name="B", kind="class", path="a/B.java", start_line=1, end_line=2, nope=1)

tests/resources/java/analysis_json/v2/README.md

@@ -71,9 +71,11 @@ dataflow structure (2,358 `ddg`, 2,038 `ssa`, 320 `points-to`, 20 self-loops, 1,
 `cdg`, 76 `summary`, 247 `call_graph`, 258 `param_in`, 97 `param_out`), as well as the signature
 table further down (154 / 0 / 45 of 45 for `a1`, 8 / 4 / 4 of 5 for `a4`).
 
-Note that the new fields make a 3.1.0 payload **unparsable by the pre-#369 models**, which are
-`extra="forbid"`: the graph contract stays at 2.0.0 and the wire is additive, but the SDK's mirror
-had to grow the five fields before it could read one.
+Note that the new fields made a 3.1.0 payload **unparsable by the pre-#369 models**, which were
+`extra="forbid"` at the time: the graph contract stays at 2.0.0 and the wire is additive, but the
+SDK's mirror had to grow the five fields before it could read one. Since #386 the mirrors are
+`extra="ignore"`, so an additive release no longer fails to parse -- it is consumable immediately,
+and an undeclared field is simply unreachable until someone declares it.
 
 ## What the 3.0.3 regeneration moved
 

tests/resources/typescript/analysis_json/v2/README.md

@@ -31,7 +31,8 @@ Measured on `a4`: `param_in` went from 5 of 31 edges carrying `var` to 31 of 31,
 model needed widening, because `TSParamEdge` had already declared `var` as optional. The sibling
 analyzers shipped the same fix in lockstep (codeanalyzer-python 1.5.1 / #196, codeanalyzer-java
 3.1.2 / #250), and Java's did need a model widening: `JParamEdge` had only `src`/`dst`, so every
-param edge failed `extra="forbid"` until the field was added.
+param edge failed validation until the field was added — the mirrors were `extra="forbid"` then, and
+are `extra="ignore"` since #386, so an addition like this one is now absorbed silently instead.
 
 Why it mattered: the schemas had declared the property since the L4 layer landed and the projections
 wrote nothing, so a consumer predicate on `var` was `null` on every edge crossing a call boundary.