Skip to content

Commit 4e757c8

Browse files
dmealingclaude
andcommitted
fix(render,verify): close six divergences review found in the has-accessor change
Code review over the branch found that the change meant to END a cross-port divergence had introduced four of its own, plus two verify defects. Fixing them is the point; the lesson is that "one shared rule" written four times in four languages is still four implementations. A LIVE REGRESSION, worst first. TS rebuilt ANY non-array object from Object.entries(), so anything carrying its own prototype was flattened: a Date rendered as "[object Object]" and a class instance lost its getters. Confirmed by running it. Now only PLAIN objects are rebuilt (prototype is Object.prototype or null), which is also what the other ports do. THE FOUR PORTS DID NOT AGREE: - C# an IDictionary is IEnumerable, so it matched the collection arm and an empty nested object reported ABSENT where every other port says present. Dictionary is now matched first. - Java recursion covered Map/List only, so Set elements and array elements got no accessors -- contradicting the method's own javadoc; and an empty primitive array reported PRESENT because Object[] does not match int[]. Now every Collection recurses, arrays recurse by reflection, and length is read with Array.getLength. - Py the numeric skip listed int/float/complex, missing Decimal and Fraction, so Python injected hasPrice where Java and C# inject nothing. Now numbers.Number. TWO VERIFY DEFECTS: - The unclassified-file fail-open downgraded a broken @Verifiedby claim to a warning if the name appeared ANYWHERE in production source -- including the mountCrudRoutes case this file's own header cites as why the comment-only check exists. The downgrade now requires the file to look like a test by LOCATION or by CONTENT; production source with a matching name satisfies neither and stays a hard error. - The bare-name fallback for non-object claims reached into any package where the name was unique. resolveObjectRef's own bare fallback is root-level only; this now matches it. GATED, not just fixed. The shared conformance fixture gains the empty-nested-object case that catches the C# arm, so all five ports pin it rather than three agreeing by luck. TS gains four regression tests (Date renders as a Date, class getters survive, a Date-valued field still derives, nested plain objects still derive). Verified after: TS render 327, cli 20 targeted; C# 291; Python 257; JVM render green (snapshot regenerated for the extended fixture). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 6c06603 commit 4e757c8

11 files changed

Lines changed: 98 additions & 14 deletions

File tree

‎fixtures/render-conformance/render-derived-has-accessor/expected.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ title:Party
22
bio: (none)
33
sponsor: Guild
44
companions: (none)
5-
abilities: Fireball[fire aoe ] Mend[untagged]
5+
abilities: Fireball[fire aoe ] Mend[untagged]
6+
details: present

‎fixtures/render-conformance/render-derived-has-accessor/payload.json‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,21 @@
22
"title": "Party",
33
"bio": " ",
44
"abilities": [
5-
{ "name": "Fireball", "tags": ["fire", "aoe"] },
6-
{ "name": "Mend", "tags": [] }
5+
{
6+
"name": "Fireball",
7+
"tags": [
8+
"fire",
9+
"aoe"
10+
]
11+
},
12+
{
13+
"name": "Mend",
14+
"tags": []
15+
}
716
],
817
"companions": [],
9-
"sponsor": { "name": "Guild" }
10-
}
18+
"sponsor": {
19+
"name": "Guild"
20+
},
21+
"emptyDetails": {}
22+
}

‎fixtures/render-conformance/render-derived-has-accessor/template.mustache‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ title:{{title}}
22
bio:{{#hasBio}} {{bio}}{{/hasBio}}{{^hasBio}} (none){{/hasBio}}
33
sponsor:{{#hasSponsor}} {{sponsor.name}}{{/hasSponsor}}
44
companions:{{#hasCompanions}} some{{/hasCompanions}}{{^hasCompanions}} (none){{/hasCompanions}}
5-
abilities:{{#hasAbilities}}{{#abilities}} {{name}}{{#hasTags}}[{{#tags}}{{.}} {{/tags}}]{{/hasTags}}{{^hasTags}}[untagged]{{/hasTags}}{{/abilities}}{{/hasAbilities}}
5+
abilities:{{#hasAbilities}}{{#abilities}} {{name}}{{#hasTags}}[{{#tags}}{{.}} {{/tags}}]{{/hasTags}}{{^hasTags}}[untagged]{{/hasTags}}{{/abilities}}{{/hasAbilities}}
6+
details:{{#hasEmptyDetails}} present{{/hasEmptyDetails}}{{^hasEmptyDetails}} absent{{/hasEmptyDetails}}

‎server/csharp/MetaObjects.Render/PayloadAccessors.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ public static bool IsBooleanAccessor(List<IReadOnlyList<PayloadField>> stack, st
7676
case bool: return null;
7777
case sbyte or byte or short or ushort or int or uint or long or ulong
7878
or float or double or decimal: return null;
79+
// A dictionary IS IEnumerable, so it must be matched FIRST — otherwise an
80+
// empty nested object reports absent here and present in every other port.
81+
case System.Collections.IDictionary: return true;
7982
case System.Collections.IEnumerable seq:
8083
{
8184
foreach (var _ in seq) return true;

‎server/java/render/src/main/java/com/metaobjects/render/PayloadAccessors.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,11 @@ public static Boolean accessorValue(Object value) {
6161
if (value == null) return Boolean.FALSE;
6262
if (value instanceof CharSequence cs) return !cs.toString().isBlank();
6363
if (value instanceof Boolean || value instanceof Number) return null;
64+
if (value instanceof java.util.Map<?, ?>) return Boolean.TRUE;
6465
if (value instanceof java.util.Collection<?> c) return !c.isEmpty();
65-
if (value instanceof Object[] a) return a.length > 0;
66+
// getLength covers primitive arrays too; Object[] alone reported an empty
67+
// int[] as present, where every other port reports absent.
68+
if (value.getClass().isArray()) return java.lang.reflect.Array.getLength(value) > 0;
6669
return Boolean.TRUE;
6770
}
6871

@@ -101,9 +104,17 @@ private static Object withDerivedAccessors(Object payload, int depth) {
101104
}
102105
return out;
103106
}
104-
if (payload instanceof java.util.List<?> list) {
105-
java.util.List<Object> out = new java.util.ArrayList<>(list.size());
106-
for (Object item : list) out.add(withDerivedAccessors(item, depth + 1));
107+
// Every Collection, not just List — a Set's elements are contexts too, which
108+
// this method's own contract promises.
109+
if (payload instanceof java.util.Collection<?> coll) {
110+
java.util.List<Object> out = new java.util.ArrayList<>(coll.size());
111+
for (Object item : coll) out.add(withDerivedAccessors(item, depth + 1));
112+
return out;
113+
}
114+
if (payload.getClass().isArray() && !payload.getClass().getComponentType().isPrimitive()) {
115+
int n = java.lang.reflect.Array.getLength(payload);
116+
java.util.List<Object> out = new java.util.ArrayList<>(n);
117+
for (int i = 0; i < n; i++) out.add(withDerivedAccessors(java.lang.reflect.Array.get(payload, i), depth + 1));
107118
return out;
108119
}
109120
return payload;

‎server/java/render/src/test/resources/snapshots/render-derived-has-accessor.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@ title:Party
22
bio: (none)
33
sponsor: Guild
44
companions: (none)
5-
abilities: Fireball[fire aoe ] Mend[untagged]
5+
abilities: Fireball[fire aoe ] Mend[untagged]
6+
details: present

‎server/python/src/metaobjects/render/payload_accessors.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
from __future__ import annotations
1818

19+
import numbers
1920
from collections.abc import Mapping, Sequence
2021
from typing import Any
2122

@@ -71,7 +72,7 @@ def accessor_value(value: Any) -> bool | None:
7172
if isinstance(value, str):
7273
return bool(value.strip())
7374
# bool before int — bool IS an int in Python, and a boolean field gets no accessor.
74-
if isinstance(value, (bool, int, float, complex)):
75+
if isinstance(value, numbers.Number):
7576
return None
7677
if isinstance(value, Mapping):
7778
return True

‎server/typescript/packages/cli/src/lib/requirement-check.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,9 @@ function resolveClaimTarget(root: MetaData, owner: string, referrerPkg: string):
183183
// at the wrong thing without anyone noticing.
184184
const local = referrerPkg === "" ? [] : candidates.filter((c) => c.resolutionKey() === `${referrerPkg}${PACKAGE_SEPARATOR}${owner}`);
185185
if (local.length === 1) return local[0];
186-
const bare = candidates.filter((c) => c.name === owner);
186+
// Root-level (unpackaged) only, matching resolveObjectRef's own bare fallback. A bare
187+
// ref must not reach into an arbitrary package just because the name is unique there.
188+
const bare = candidates.filter((c) => c.name === owner && c.resolutionKey() === owner);
187189
return bare.length === 1 ? bare[0] : undefined;
188190
}
189191

‎server/typescript/packages/cli/src/lib/verified-by-scan.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,20 @@ function walk(
183183
* per run. Returns the first unclassified source file containing the name, which is
184184
* enough to tell the author which pattern they are missing.
185185
*/
186+
/** Does this path or body look like a test the corpus definition simply did not match?
187+
* Deliberately narrow: living under a test directory, or containing an assertion/test
188+
* declaration. Without this, a name occurring anywhere in PRODUCTION source downgrades a
189+
* genuinely broken claim to a warning — the exact failure the comment-only check exists
190+
* to catch. */
191+
const TESTISH_PATH = /(^|\/)(tests?|spec|__tests__|src\/test)(\/|$)/i;
192+
const TESTISH_BODY = /\b(assert\w*|expect|should|@Test|def test_|it\(|test\(|describe\()/;
193+
194+
/** Test by LOCATION or by CONTENT — either is enough. Production source with a matching
195+
* name satisfies neither, which is the case that must stay a hard error. */
196+
function looksLikeTest(rel: string, lines: string[]): boolean {
197+
return TESTISH_PATH.test(rel) || lines.some((l) => TESTISH_BODY.test(l));
198+
}
199+
186200
function findOutsideCorpus(name: string, root: string, files: string[]): string | undefined {
187201
const rx = wordRx(name);
188202
for (const rel of files) {
@@ -192,7 +206,7 @@ function findOutsideCorpus(name: string, root: string, files: string[]): string
192206
const lines = readFileSync(abs, "utf8").split("\n");
193207
for (let i = 0; i < lines.length; i++) {
194208
const line = lines[i] ?? "";
195-
if (rx.test(line) && !isCommentLine(line, rel)) return rel;
209+
if (rx.test(line) && !isCommentLine(line, rel) && looksLikeTest(rel, lines)) return rel;
196210
}
197211
} catch {
198212
/* unreadable file is not a finding */

‎server/typescript/packages/render/src/payload-accessors.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,12 @@ export function withDerivedAccessors<T>(payload: T, depth = 0): T {
7777
return payload.map((v) => withDerivedAccessors(v, depth + 1)) as unknown as T;
7878
}
7979
if (payload === null || typeof payload !== "object") return payload;
80+
// PLAIN objects only. Rebuilding from Object.entries() would flatten anything with
81+
// its own prototype — a Date stringifies to "[object Object]" and a class instance
82+
// loses its getters — and the other ports only rebuild map-shaped values, so
83+
// rebuilding more here would be a divergence as well as a regression.
84+
const proto = Object.getPrototypeOf(payload);
85+
if (proto !== Object.prototype && proto !== null) return payload;
8086

8187
const src = payload as Record<string, unknown>;
8288
const out: Record<string, unknown> = {};

0 commit comments

Comments
 (0)