Skip to content

Commit bebc6e4

Browse files
dmealingclaude
andcommitted
fix(conformance): match the reference's content-sorted spec resolution order; retire the vacuous unknown-keys case name
F12 — Java/C#/Python's SourceResolver.resolveSources processed declared source specs in DECLARED order, while each port's own comment claimed to mirror the TypeScript reference's `orderedPathSpecs` (`sources.ts`), which kind-validates in declared order but then resolves in CONTENT order (`JSON.stringify(spec)`, ascending — for a validated `path`-only spec this reduces to an ordinal sort of the path string). The three ports' comments were half-true: they implemented the whole-list-kind-validation half but not the sort. With two simultaneously-unresolvable declared paths, TS's ERR_SOURCE_UNRESOLVED names the content-first one; the other three named whichever was declared first instead. Verified this changes ONLY which of several unresolvable paths gets named in the raised error, never the resolved file SET: de-duplication into the result is order-independent by construction (a Map/LinkedHashSet/HashSet keyed on normalized path), and file order was already outside the cross-port contract per the corpus README. All three now sort the validated path specs ordinally before Pass 2 (matching JS's UTF-16 code-unit string comparison — Java's `Collections.sort` on `String`, C#'s `StringComparer.Ordinal`, Python's `sorted(..., key=...)` on `str` all agree with it for the ASCII paths in scope). A new unit test per port (mirroring an empirical probe against the TS reference) declares two unresolvable paths out of content order and asserts the content-first one is named; reverted each fix, confirmed the wrong path was named (RED), restored, confirmed the content-first one is (GREEN). Full per-port suites (Python 1700, Java 1476, C# 934) stay green, so no other behavior depends on the prior declared-order processing. F13 — the `unknown-top-level-keys-are-ignored` corpus case is vacuous for TypeScript: all four keys it supplies (`pending_in_git`, `confidence_thresholds`, `extract`, `migrate`) are TS's OWN recognized top-level config keys, so TS's `ConfigSchema` (`config.ts`, `.strict()`) passes the case by RECOGNIZING them, not by ignoring an unknown key — the other three ports genuinely don't know these keys and correctly ignore them, but the case can't tell that apart from TS's different reason for the same outcome. Renamed to `typescript-owned-top-level-keys-do-not-affect-source-resolution` (inputs unchanged — not weakened) and the README section rewritten to state the narrower, TRUE claim precisely. A genuinely unrecognized key (e.g. `"foo": 1`, unknown to all four ports) IS a real, confirmed cross-port divergence, verified empirically: `resolveCollection` → `loadConfig` → `ConfigSchema.parse` is `.strict()` at the top level, so an unrecognized key throws a ZodError before source resolution is ever reached, while Java/C#/Python all resolve successfully, silently ignoring it. NOT added as a shared corpus case and NOT fixed: doing either would mean changing the reference implementation (`config.ts`/`collection.ts`, explicitly out of scope) — loosening `ConfigSchema`'s top-level strictness has a blast radius well beyond source resolution (every `loadConfig` caller), which is a deliberate call for a human to make, not one this pass should make unilaterally. Documented in the README as an open, human-reviewable follow-up rather than silently dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7513716 commit bebc6e4

8 files changed

Lines changed: 194 additions & 6 deletions

File tree

fixtures/source-resolution-conformance/README.md

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,36 @@ README.md
9393
loop) reports whichever error comes first in declaration order instead,
9494
diverging on exactly one of the two cases depending on which order it
9595
happens to process first.
96-
- **Unknown top-level config keys are IGNORED.** The file carries
97-
TypeScript-owned keys no other port models. `schema_version` and `sources` are
98-
the neutral subset; each port validates those strictly and ignores the rest.
96+
- **A TypeScript-owned top-level key does not affect source resolution in any
97+
port.** `schema_version` and `sources` are the neutral subset every port
98+
models; `pending_in_git` / `confidence_thresholds` / `extract` / `migrate`
99+
are TypeScript's own, and `typescript-owned-top-level-keys-do-not-affect-
100+
source-resolution` pins that their presence resolves the same file set
101+
everywhere. Read that case name literally — it is narrower than "unknown
102+
keys are ignored" on purpose. Those four keys are UNKNOWN to Java/C#/Python
103+
(which ignore any key outside `schema_version`/`sources`, by design) but
104+
KNOWN to TypeScript's own `ConfigSchema` (`sdk/src/config.ts`), which
105+
recognizes and validates them as part of its own project state. A case
106+
built only from keys TS recognizes cannot tell "TS ignored this because it
107+
doesn't affect resolution" apart from "TS ignored this because it doesn't
108+
affect resolution AND happened to also validate it" — the two are
109+
indistinguishable from the outside, and only the first is what every other
110+
port's "ignore the rest" behavior demonstrates.
111+
**A genuinely unrecognized key (e.g. `"foo": 1`, unknown to all four ports)
112+
is a real, confirmed, cross-port DIVERGENCE, not covered by this corpus.**
113+
Verified empirically: `resolveCollection` (`collection.ts`) calls
114+
`loadConfig`, which parses the WHOLE file through `ConfigSchema.parse`
115+
`.strict()` at the top level (`config.ts`) — so a key no version of
116+
TypeScript has ever declared throws a `ZodError` and resolution never
117+
reaches the source-listing step at all, while Java/C#/Python all resolve
118+
successfully, silently ignoring it. Not added as a shared `expectFiles`
119+
case here because doing so would need EITHER loosening `ConfigSchema`'s
120+
top-level strictness (a reference-implementation behavior change with a
121+
blast radius well beyond source resolution — every `loadConfig` caller,
122+
not just this corpus) OR asserting a `true`-sentinel `expectError` that
123+
TypeScript alone would satisfy, contradicting the other three ports'
124+
actual success — neither of which this corpus is positioned to decide
125+
unilaterally. Left as an open, human-reviewable follow-up.
99126

100127
## Order is deliberately NOT pinned
101128

fixtures/source-resolution-conformance/cases.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@
178178
"expectError": "ERR_COLLECTION_NOT_FOUND"
179179
},
180180
{
181-
"name": "unknown-top-level-keys-are-ignored",
181+
"name": "typescript-owned-top-level-keys-do-not-affect-source-resolution",
182182
"tree": {
183183
"model/meta.a.json": "{\"metadata.root\":{\"children\":[]}}"
184184
},
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System.Collections.Generic;
2+
using System.IO;
3+
using MetaObjects;
4+
using MetaObjects.Config;
5+
using Xunit;
6+
7+
namespace MetaObjects.Conformance.Tests;
8+
9+
/// <summary>
10+
/// Focused unit coverage for <see cref="SourceResolver.ResolveSources"/> shapes
11+
/// not gated by the shared <c>source-resolution-conformance</c> corpus.
12+
/// </summary>
13+
public sealed class SourceResolverTests
14+
{
15+
// F12 — Pass 2 resolves in CONTENT order (ordinal path-string sort), not
16+
// declared order, mirroring the TypeScript reference's `orderedPathSpecs`
17+
// (`sources.ts`: kind-validated, then sorted by `JSON.stringify(spec)`,
18+
// which for a validated `path`-only spec reduces to the path string alone
19+
// — verified empirically: `resolveSources(dir, [{path:"zzz-missing"},
20+
// {path:"aaa-missing"}])` names "aaa-missing", the content-first one, even
21+
// though "zzz-missing" is declared first). With BOTH paths unresolvable,
22+
// only a port that content-sorts before Pass 2 names "aaa-missing" here;
23+
// a declared-order implementation names "zzz-missing" instead.
24+
[Fact]
25+
public void TwoUnresolvablePaths_ReportsTheContentFirstOne()
26+
{
27+
var root = Path.Combine(Path.GetTempPath(), "source-resolver-order-" + System.Guid.NewGuid().ToString("N"));
28+
Directory.CreateDirectory(root);
29+
try
30+
{
31+
var specs = new List<IReadOnlyDictionary<string, string>>
32+
{
33+
new Dictionary<string, string> { ["path"] = "zzz-missing" },
34+
new Dictionary<string, string> { ["path"] = "aaa-missing" },
35+
};
36+
var ex = Assert.Throws<MetaModelException>(() => SourceResolver.ResolveSources(root, specs));
37+
Assert.Contains("aaa-missing", ex.Message);
38+
Assert.DoesNotContain("zzz-missing", ex.Message);
39+
}
40+
finally
41+
{
42+
Directory.Delete(root, recursive: true);
43+
}
44+
}
45+
}

server/csharp/MetaObjects/Config/SourceResolver.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ public static class SourceResolver
2424
/// or missing path happens to sit first — the corpus pins that an unsupported
2525
/// KIND anywhere in the list wins over an unresolved PATH regardless of which
2626
/// is declared first (`unsupported-kind-precedes-unresolved-path-when-path-is-
27-
/// declared-first`/`-second`).
27+
/// declared-first`/`-second`). Pass 2 then resolves the validated specs in
28+
/// CONTENT order (ordinal path-string sort), also matching `orderedPathSpecs`
29+
/// — not the resolved file SET (de-dup is order-independent), only which
30+
/// declared path's `ERR_SOURCE_UNRESOLVED` fires first when more than one is
31+
/// simultaneously unresolvable.
2832
public static IReadOnlyList<string> ResolveSources(
2933
string configDir,
3034
IReadOnlyList<IReadOnlyDictionary<string, string>> specs)
@@ -45,6 +49,15 @@ public static IReadOnlyList<string> ResolveSources(
4549
ErrorCode.ERR_SOURCE_KIND_UNSUPPORTED);
4650
}
4751
}
52+
// Content order (ordinal — matches the reference's UTF-16 code-unit
53+
// comparison), not declared order — mirrors `orderedPathSpecs` in
54+
// `sources.ts` (kind-validated, then sorted by `JSON.stringify(spec)`,
55+
// which for a validated `path`-only spec reduces to the path string
56+
// alone). Does not change the resolved file SET (de-dup is
57+
// order-independent); only decides which declared path's
58+
// `ERR_SOURCE_UNRESOLVED` fires first when more than one is
59+
// simultaneously unresolvable.
60+
pathSpecs.Sort(StringComparer.Ordinal);
4861

4962
// Pass 2 — resolve each validated path spec against the filesystem.
5063
var seen = new List<string>();

server/java/metadata/src/main/java/com/metaobjects/config/SourceResolver.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.nio.file.Files;
2424
import java.nio.file.Path;
2525
import java.util.ArrayList;
26+
import java.util.Collections;
2627
import java.util.LinkedHashSet;
2728
import java.util.List;
2829
import java.util.Map;
@@ -59,6 +60,16 @@ private SourceResolver() {}
5960
* an unresolved PATH regardless of which is declared first
6061
* ({@code unsupported-kind-precedes-unresolved-path-when-path-is-declared-first}/
6162
* {@code -second}).
63+
*
64+
* <p>Pass 2 processes the validated specs in CONTENT order (natural string
65+
* ordering of each spec's {@code path}), not declared order — mirroring the
66+
* reference implementation's {@code orderedPathSpecs}
67+
* ({@code sources.ts}: kind-validated, then sorted by
68+
* {@code JSON.stringify(spec)}, which for a validated {@code path}-only spec
69+
* reduces to sorting by the path string alone). This does not change the
70+
* resolved file SET — de-duplication is order-independent — only which
71+
* declared path's {@code ERR_SOURCE_UNRESOLVED} fires first when more than one
72+
* is simultaneously unresolvable, which order-independence alone cannot pin.
6273
*/
6374
public static List<Path> resolveSources(Path configDir, List<Map<String, String>> specs) {
6475
// Pass 1 — kind validation across the WHOLE set, no filesystem I/O yet.
@@ -73,6 +84,7 @@ public static List<Path> resolveSources(Path configDir, List<Map<String, String>
7384
}
7485
pathSpecs.add(rawPath);
7586
}
87+
Collections.sort(pathSpecs);
7688

7789
// Pass 2 — resolve each validated path spec against the filesystem.
7890
LinkedHashSet<Path> seen = new LinkedHashSet<>();
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Copyright 2003 Doug Mealing LLC dba Meta Objects
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.metaobjects.config;
17+
18+
import com.metaobjects.MetaDataException;
19+
import org.junit.Test;
20+
21+
import java.io.IOException;
22+
import java.nio.file.Files;
23+
import java.nio.file.Path;
24+
import java.util.LinkedHashMap;
25+
import java.util.List;
26+
import java.util.Map;
27+
28+
import static org.junit.Assert.assertThrows;
29+
import static org.junit.Assert.assertTrue;
30+
31+
/**
32+
* Focused unit coverage for {@link SourceResolver#resolveSources} shapes not gated
33+
* by the shared {@code source-resolution-conformance} corpus.
34+
*/
35+
public class SourceResolverTest {
36+
37+
private static Map<String, String> pathSpec(String path) {
38+
Map<String, String> m = new LinkedHashMap<>();
39+
m.put("path", path);
40+
return m;
41+
}
42+
43+
// F12 — Pass 2 resolves in CONTENT order (natural string ordering of each
44+
// spec's `path`), not declared order, mirroring the TypeScript reference's
45+
// `orderedPathSpecs` (`sources.ts`: kind-validated, then sorted by
46+
// `JSON.stringify(spec)`, which for a validated `path`-only spec reduces to
47+
// the path string alone — verified empirically: `resolveSources(dir,
48+
// [{path:"zzz-missing"},{path:"aaa-missing"}])` names "aaa-missing", the
49+
// content-first one, even though "zzz-missing" is declared first). With
50+
// BOTH paths unresolvable, only a port that content-sorts before Pass 2
51+
// names "aaa-missing" here; a declared-order implementation names
52+
// "zzz-missing" instead.
53+
@Test
54+
public void twoUnresolvablePathsReportsTheContentFirstOne() throws IOException {
55+
Path root = Files.createTempDirectory("source-resolver-order-");
56+
try {
57+
List<Map<String, String>> specs = List.of(pathSpec("zzz-missing"), pathSpec("aaa-missing"));
58+
MetaDataException ex = assertThrows(MetaDataException.class,
59+
() -> SourceResolver.resolveSources(root, specs));
60+
assertTrue("expected \"aaa-missing\" (content-first) in: " + ex.getMessage(),
61+
ex.getMessage().contains("aaa-missing"));
62+
assertTrue("must NOT name \"zzz-missing\" (declared-first, content-second): " + ex.getMessage(),
63+
!ex.getMessage().contains("zzz-missing"));
64+
} finally {
65+
Files.delete(root);
66+
}
67+
}
68+
}

server/python/src/metaobjects/config/source_resolver.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,18 @@ def resolve_sources(config_dir: Path, specs: list[dict[str, str]]) -> list[Path]
7272
# Whole-list kind validation FIRST — see `_validate_kinds`.
7373
_validate_kinds(specs)
7474

75+
# Resolve in CONTENT order (ordinal path-string sort), not declared order —
76+
# mirrors `orderedPathSpecs` in `sources.ts` (kind-validated, then sorted by
77+
# `JSON.stringify(spec)`, which for a validated `path`-only spec reduces to
78+
# the path string alone). Does not change the resolved file SET (the `seen`
79+
# de-dup below is order-independent); only decides which declared path's
80+
# `ERR_SOURCE_UNRESOLVED` fires first when more than one is simultaneously
81+
# unresolvable.
82+
ordered_specs = sorted(specs, key=lambda s: s["path"])
83+
7584
seen: dict[Path, None] = {}
7685

77-
for spec in specs:
86+
for spec in ordered_specs:
7887
raw = Path(spec["path"])
7988
target = raw if raw.is_absolute() else (config_dir / raw)
8089

server/python/tests/config/test_source_resolver.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,20 @@ def test_kind_error_precedes_unresolved_path_regardless_of_order(tmp_path: Path)
6565
assert e_resource_first.value.code == ErrorCode.ERR_SOURCE_KIND_UNSUPPORTED
6666

6767

68+
def test_two_unresolvable_paths_reports_the_content_first_one(tmp_path: Path) -> None:
69+
# F12 — Pass 2 resolves in CONTENT order (ordinal path-string sort), not
70+
# declared order, mirroring the TypeScript reference's `orderedPathSpecs`
71+
# (verified empirically: `resolveSources(dir, [{path:"zzz-missing"},
72+
# {path:"aaa-missing"}])` names "aaa-missing", the content-first one, even
73+
# though "zzz-missing" is declared first). With BOTH paths unresolvable,
74+
# only the port that content-sorts before Pass 2 names "aaa-missing" here;
75+
# a declared-order implementation would name "zzz-missing" instead.
76+
with pytest.raises(ParseError) as e:
77+
resolve_sources(tmp_path, [{"path": "zzz-missing"}, {"path": "aaa-missing"}])
78+
assert "aaa-missing" in str(e.value)
79+
assert "zzz-missing" not in str(e.value)
80+
81+
6882
def test_collection_falls_back_to_default_dir(tmp_path: Path) -> None:
6983
(tmp_path / "metaobjects").mkdir()
7084
(tmp_path / "metaobjects" / "a.json").write_text("{}")

0 commit comments

Comments
 (0)