Skip to content

Commit e529a61

Browse files
dmealingclaude
andcommitted
fix(python,sdk,metadata): the nudge blind to 1.0, and three rules with a second spelling
The Python agent-context nudge dropped the leading MAJOR from both of its version coordinates, borrowing the Java port's rationale verbatim. That rationale is explicitly "a fact peculiar to the JVM": Maven Central carries a historical major of npm-major + 7, so 7.24.1 and 0.24.1 genuinely name one release. Nothing of the kind is true here — `generatedBy` is an npm version stamped by the Node CLI, `installed_metaobjects_version()` reads the PyPI version, and npm and PyPI share the major by policy. So 1.0.0 and 2.0.0 compared EQUAL, and — the live case — a Python install upgraded to 1.0.0 whose context was scaffolded at 0.25.0 got NO advisory at all: the ordering helper compared (25,0) to (0,0), concluded the 0.25.0 context was newer than the 1.0.0 install, and took the publish-what-changed exemption. That is the one upgrade the feature exists to catch, and every Python adopter hits it the moment 1.0 ships. The docstring's own "known bound" was also not the "suppressed once" it claimed — nothing changes the state, so it was permanent. Both coordinates carry the major now, and the ordering coordinate is DERIVED from the equality one instead of parsed by a second regex — differential-checked over 11k inputs as identical on orderability and on (minor, patch). PEP 440 spelling equivalence and the publish-what-changed exemption are unchanged. REMOVED the sdk's `.meta` path helpers, finishing what took the forge record store. `recordPath` built `<metaRoot>/memory/<type>/<id>.json` — that store's exact layout. `resolveMetaRoot` walked up for a `.meta` dir `meta init` does not scaffold and this package's own test asserts is NOT created. Its only caller anywhere was the sdk README example, wrong twice: it threw before reaching `loadConfig`, and `loadConfig` reads `<dir>/config.json` while a project's config lives under `.metaobjects/`. The example now shows `resolveCollection` feeding `loadMemory`, verified by RUNNING it verbatim against a freshly `meta init`-ed project. Also corrected a comment claiming the removed storage layer "had no test at all" — the same commit that wrote that sentence deleted its 217-line test. Three rules that had a second, unexercised spelling: - the authored-`<type>.base` refusal, written twice with a byte-identical `why` and migration path in the JSON and YAML rewriter arms. Detection legitimately differs and stays per-arm; the verdict has one source now. - `mavenVersion` inlined the major offset instead of calling `mavenMajor` — in the file that exists because that offset had four doors and two were wrong. It made a fifth, and left `mavenMajor` with no production caller at all. - `release-verify.mjs` reassembled VERSION from the captures of the match it had just made on VERSION. - Python's `gen_state_dir_for` re-derived the project root rather than calling `project_root_for`, whose docstring already claimed it "is anchored here". python 1998 pass; metadata 2621 pass; sdk 273 pass; scripts typecheck + maven coordinate tests green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent 6e2a339 commit e529a61

12 files changed

Lines changed: 249 additions & 158 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,71 @@ integration cases generated with an explicit `--out <dir>/docs` while `verify --
7272
the config default, and passed only because the two coincided. Sixteen cases failed the moment
7373
they stopped. They now take the config path, which is what the gate reads.
7474

75+
### Fixed — the Python nudge was blind to the 1.0 cut it exists to catch
76+
77+
`agent_context_staleness` on the Python port dropped the leading MAJOR from both of its
78+
version coordinates, borrowing the Java port's rationale verbatim. That rationale is
79+
explicitly "a fact peculiar to the JVM": Maven Central carries a historical major of
80+
npm-major + 7, so `7.24.1` and `0.24.1` genuinely name one release and the coordinate must
81+
ignore the major to see it. Nothing of the kind is true here — `generatedBy` is stamped by
82+
the Node CLI (an npm version), `installed_metaobjects_version()` reads the PyPI distribution
83+
version, and npm and PyPI share the major by policy.
84+
85+
So `1.0.0` and `2.0.0` compared EQUAL, and — the live case — a Python install upgraded to
86+
`1.0.0` whose agent context was scaffolded at `0.25.0` got **no advisory at all**: the
87+
ordering helper compared `(25, 0)` against `(0, 0)`, concluded the 0.25.0 context was NEWER
88+
than the 1.0.0 install, and took the publish-what-changed exemption. That is the single
89+
upgrade the whole feature exists to catch, and every Python adopter would have hit it the
90+
moment 1.0 shipped. The "known bound" the docstring owned — a `0.24.x` context against a
91+
`1.0.0` install reading as ahead — was also not the "suppressed once" it claimed: nothing
92+
changes the state, so the suppression was permanent.
93+
94+
Both coordinates now carry the major, and the ordering coordinate is DERIVED from the
95+
equality one rather than parsed by a second regex — differential-checked over 11k inputs as
96+
identical on orderability and on `(minor, patch)`. The PEP 440 spelling equivalence
97+
(`1.0.0rc5` == `1.0.0-rc.5`) and the publish-what-changed exemption are unchanged; the
98+
exemption now applies across majors too, which is correct, since the canonical scaffolder is
99+
npm and a genuinely newer stamp must still not nudge.
100+
101+
### Removed — the sdk's `.meta` path helpers, and a README example that could only throw
102+
103+
`recordPath` and `resolveMetaRoot` are gone, finishing the removal that took the forge memory
104+
record schemas and their storage layer. `recordPath` built `<metaRoot>/memory/<type>/<id>.json`
105+
— the layout of exactly that deleted store. `resolveMetaRoot` walked up for a `.meta`
106+
directory, which `meta init` does not scaffold and this package's own test asserts is NOT
107+
created.
108+
109+
Its only caller anywhere was the sdk README's usage example, which was wrong twice: it would
110+
throw `no .meta directory found` before reaching `loadConfig`, and `loadConfig` reads
111+
`<dir>/config.json` while a project's config lives under `.metaobjects/`. The example now
112+
shows `resolveCollection` — the documented single authority for locating a project — feeding
113+
`loadMemory`, and was verified by running it verbatim against a freshly `meta init`-ed
114+
project rather than by reading it.
115+
116+
Also corrected: a comment in `sdk/src/index.ts` claimed the removed storage layer "had no
117+
test at all". The same commit that wrote that sentence deleted `sdk/test/storage.test.ts`,
118+
217 lines of it.
119+
120+
### Fixed — three rules that had a second, unexercised spelling
121+
122+
- The authored-`<type>.base` refusal was written twice, in the JSON and YAML arms of
123+
`meta upgrade`'s rewriter, with a byte-identical `why` and migration path. The DETECTION
124+
legitimately differs (JSON key ranges vs YAML node spans) and stays per-arm; the verdict an
125+
adopter reads is one decision and now has one source, with only the line number supplied
126+
per arm.
127+
- `mavenVersion` inlined `Number(major) + MAVEN_MAJOR_OFFSET` instead of calling `mavenMajor`
128+
— in the very file that exists because that offset had four doors and two of them were
129+
wrong. It made a fifth, and left `mavenMajor` with **no production caller at all**: an
130+
exported rule exercised only by its own unit test, sitting beside a copy of itself that
131+
every release actually ran.
132+
- `release-verify.mjs` reassembled `VERSION` from the captures of the match it had just made
133+
on `VERSION`, giving the RC suffix a second hand-written spelling to get wrong. It passes
134+
the string through.
135+
- Python's `gen_state_dir_for` re-derived `Path(metadata_dir).resolve().parent` rather than
136+
calling `project_root_for`, whose own docstring already claimed the former "is anchored
137+
here". The manifest KEY and the manifest LOCATION have to agree on where the project is, or
138+
a lookup finds nothing and every file reads as "not ours".
139+
75140
### Fixed — four checks that could not fire, and the duplication behind two of them
76141

77142
Each of these was GREEN and each was inert. They came out of the review pass over the RC

‎scripts/maven-coordinate.mjs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,9 @@ export function mavenVersion(npmVersion) {
4747
const s = String(npmVersion);
4848
const m = /^(\d+)\.(\d+)\.(\d+)(-.+)?$/.exec(s);
4949
if (!m) throw new Error(`maven-coordinate: "${s}" is not a three-part version`);
50-
return `${Number(m[1]) + MAVEN_MAJOR_OFFSET}.${m[2]}.${m[3]}${m[4] ?? ""}`;
50+
// Through `mavenMajor`, not `Number(m[1]) + MAVEN_MAJOR_OFFSET` again. This file exists
51+
// because the offset had four doors and two were wrong; inlining it here made a fifth,
52+
// and left `mavenMajor` with no production caller at all — an exported rule whose only
53+
// exercise was its own unit test, beside a copy of itself that every release actually ran.
54+
return `${mavenMajor(s)}.${m[2]}.${m[3]}${m[4] ?? ""}`;
5155
}

‎scripts/release-verify.mjs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ const parts = VERSION?.match(/^(\d+)\.(\d+)\.(\d+)(?:-rc\.(\d+))?$/);
4545
const IS_RC = Boolean(parts?.[4]);
4646
const PYPI_VERSION = parts && `${parts[1]}.${parts[2]}.${parts[3]}${IS_RC ? `rc${parts[4]}` : ""}`;
4747
const MVN = args.find((a) => a.startsWith("--maven="))?.slice("--maven=".length)
48-
?? (parts && mavenVersion(`${parts[1]}.${parts[2]}.${parts[3]}${IS_RC ? `-rc.${parts[4]}` : ""}`));
48+
// `VERSION` itself, not its captures reassembled: `parts` came from matching VERSION
49+
// against that exact grammar, so the reconstruction could only ever equal what it was
50+
// built from — while giving the RC suffix a second, hand-written spelling to get wrong.
51+
?? (parts && mavenVersion(VERSION));
4952

5053
// A pre-release is published to `next` and must NOT move `latest` — checking `latest`
5154
// against an RC would fail every correct RC cut, and the mistake it is really guarding

‎server/python/src/metaobjects/agent_context/scaffold.py‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ def from_json(data: dict[str, object]) -> "Manifest":
8080
# The sentinel a port stamps when it cannot resolve its own installed version.
8181
_UNRESOLVED_VERSION = "0.0.0"
8282

83-
_RELEASE_RE = re.compile(r"^(\d+)\.(\d+)\.(\d+)$")
84-
8583
#: A release with an optional PRE-RELEASE segment, in either spelling one ecosystem uses.
8684
#: npm/NuGet/Maven write ``1.0.0-rc.5``; PEP 440 writes ``1.0.0rc5``. Both are the SAME
8785
#: release, and comparing their SPELLINGS is what made the nudge unsatisfiable — see
@@ -104,13 +102,19 @@ def from_json(data: dict[str, object]) -> "Manifest":
104102
}
105103

106104

107-
def _release_coordinate(version: object) -> tuple[int, int, str, int] | None:
108-
"""``(minor, patch, pre_label, pre_number)`` for a version in ANY ecosystem spelling.
105+
def _release_coordinate(version: object) -> tuple[int, int, int, str, int] | None:
106+
"""``(major, minor, patch, pre_label, pre_number)`` for a version in ANY ecosystem
107+
spelling.
109108
110-
The MAJOR is dropped for the same reason ``_release_series`` drops it: it is a
111-
per-registry constant, not information (npm/PyPI/NuGet ship ``<M>.<m>.<p>`` and Maven
112-
Central the same ``<m>.<p>`` on a major seven higher). ``minor.patch`` plus the
113-
pre-release IS the shared release coordinate.
109+
The MAJOR is INFORMATION on this port, and both helpers here used to drop it. That
110+
reduction is correct on the JVM and only there: Maven Central carries a historical
111+
major of npm-major + 7 by design, so ``7.24.1`` and ``0.24.1`` genuinely name one
112+
release and the coordinate must ignore the major to see it. Nothing of the kind holds
113+
here — ``generatedBy`` is stamped by the Node CLI (an npm version) and
114+
:func:`installed_metaobjects_version` reads the PyPI distribution version, and npm and
115+
PyPI share the major by policy. Dropping it made ``1.0.0`` and ``2.0.0`` compare EQUAL,
116+
and made a ``0.25.0``-stamped context read as NEWER than a ``1.0.0`` install — so the
117+
1.0 upgrade, the single upgrade this advisory exists to catch, produced no nudge.
114118
115119
The pre-release label is lower-cased and its separators dropped, so ``-rc.5``,
116120
``rc5`` and ``-RC5`` all reduce to ``("rc", 5)``. A final release sorts ABOVE every
@@ -133,7 +137,13 @@ def _release_coordinate(version: object) -> tuple[int, int, str, int] | None:
133137
number = int(m.group(5)) if m.group(5) is not None else 0
134138
# A FINAL release is newer than every pre-release of the same number. "~" sorts above
135139
# every ASCII letter, so the sentinel does that with plain tuple comparison.
136-
return (int(m.group(2)), int(m.group(3)), label if label else "~", number)
140+
return (
141+
int(m.group(1)),
142+
int(m.group(2)),
143+
int(m.group(3)),
144+
label if label else "~",
145+
number,
146+
)
137147

138148

139149
def _same_release(generated_by: object, current_version: str) -> bool:
@@ -156,28 +166,26 @@ def _same_release(generated_by: object, current_version: str) -> bool:
156166
return stamped is not None and stamped == installed
157167

158168

159-
def _release_series(version: object) -> tuple[int, int] | None:
160-
"""Ordered release coordinate ``(minor, patch)``, or ``None`` when not orderable.
169+
def _release_series(version: object) -> tuple[int, int, int] | None:
170+
"""Ordered release coordinate ``(major, minor, patch)``, or ``None`` when not orderable.
161171
162-
The MAJOR is deliberately dropped. It is a per-registry constant, not information:
163-
npm/PyPI/NuGet ship ``0.<m>.<p>`` and Maven Central the same ``<m>.<p>`` on its
164-
historical major ``7``, so minor.patch IS the shared release coordinate across all
165-
four ports.
172+
Derived from :func:`_release_coordinate` rather than parsed again. These two functions
173+
used to answer the same grammar with two different regexes — differential-checked
174+
identical over 200k inputs, which is the good outcome of a coin flip, not a guarantee.
175+
The ordering coordinate simply IS the equality coordinate restricted to FINAL releases,
176+
and saying so in code means the two cannot drift apart.
166177
167178
``None`` means "not orderable, so nudge", and deliberately covers prereleases
168-
(``0.24.5-rc.1``), build metadata (``0.24.5+abc``) and the ``0.0.0`` sentinel. Each
169-
must keep nudging: an RC-scaffolded context against a final release is still worth
170-
refreshing, and an unknown install must never assert "in sync".
179+
(``0.24.5-rc.1``, ``1.0.0rc5``), build metadata (``0.24.5+abc``) and the ``0.0.0``
180+
sentinel. Each must keep nudging: an RC-scaffolded context against a final release is
181+
still worth refreshing, and an unknown install must never assert "in sync".
171182
"""
172-
if not isinstance(version, str):
173-
return None
174-
v = version.strip()
175-
if v == _UNRESOLVED_VERSION:
176-
return None
177-
m = _RELEASE_RE.match(v)
178-
if m is None:
183+
coord = _release_coordinate(version)
184+
# A pre-release carries a real label; a final carries the "~" sentinel. Only finals are
185+
# orderable here, which is what the previous release-only regex enforced by construction.
186+
if coord is None or coord[3] != "~":
179187
return None
180-
return int(m.group(2)), int(m.group(3))
188+
return coord[0], coord[1], coord[2]
181189

182190

183191
def _context_is_ahead_of_install(generated_by: object, current_version: str) -> bool:

‎server/python/src/metaobjects/cli.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -470,12 +470,17 @@ def _generate(
470470
def gen_state_dir_for(metadata_dir: str) -> str:
471471
"""Where a project's codegen hash manifest lives.
472472
473-
Anchored on the project — the metadata dir's parent, i.e. the directory holding
474-
``metaobjects/`` — never on ``Path.cwd()``, which is whatever directory the process
475-
happens to sit in and would scatter ``.metaobjects/`` into any package that merely
476-
runs a generator.
473+
Anchored on the project — never on ``Path.cwd()``, which is whatever directory the
474+
process happens to sit in and would scatter ``.metaobjects/`` into any package that
475+
merely runs a generator.
476+
477+
It resolves the project through :func:`project_root_for` rather than repeating
478+
``Path(metadata_dir).resolve().parent``. That function's own docstring already claimed
479+
this one "is anchored here" while the code spelled the rule out a second time, and the
480+
manifest key and the manifest LOCATION have to agree on where the project is or a
481+
lookup finds nothing and every file reads as "not ours".
477482
"""
478-
return str(Path(metadata_dir).resolve().parent / ".metaobjects" / ".gen-state")
483+
return str(project_root_for(metadata_dir) / ".metaobjects" / ".gen-state")
479484

480485

481486
#: The conventional declarative-template-spec file (SP-1 §4), discovered when

‎server/python/tests/unit/test_agent_context_staleness.py‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,59 @@ def test_the_unresolved_sentinel_is_never_canonicalized_into_agreement() -> None
142142
# the one case where both sides genuinely say the same unknown thing.)
143143
assert agent_context_staleness({"generatedBy": "0.0.0"}, "1.0.0rc5") is not None
144144
assert agent_context_staleness({"generatedBy": "1.0.0-rc.5"}, "0.0.0") is not None
145+
146+
147+
# ── the MAJOR is information on THIS port ────────────────────────────────────
148+
# Both helpers dropped the leading major, borrowing the Java port's rationale verbatim.
149+
# That rationale is explicitly "a fact peculiar to the JVM": Maven Central carries a
150+
# historical major of npm-major + 7, so `7.24.1` and `0.24.1` ARE one release and the
151+
# coordinate has to drop the major to compare them.
152+
#
153+
# Nothing of the kind is true here. `generatedBy` is stamped by the Node CLI (npm) and
154+
# `installed_metaobjects_version()` reads the PyPI distribution version — and npm and PyPI
155+
# share the major by policy (both go to 1.0.0 at the cut). So on this port the major is
156+
# real information, and dropping it made the nudge assert agreement across a whole major.
157+
158+
159+
def test_a_context_a_MAJOR_behind_is_not_in_sync() -> None:
160+
# `1.0.0` vs `2.0.0` reduced to the same (0, 0) coordinate and read as one release.
161+
assert agent_context_staleness({"generatedBy": "1.0.0"}, "2.0.0") is not None
162+
assert agent_context_staleness({"generatedBy": "0.25.0"}, "1.25.0") is not None
163+
164+
165+
def test_the_1_0_cut_itself_nudges_a_0_x_context() -> None:
166+
# The live case at the cut, and the reason this is not hypothetical: a Python install
167+
# upgraded to 1.0.0 whose agent context was scaffolded at 0.25.0 got NO advisory at
168+
# all. `_context_is_ahead_of_install` compared (25, 0) against (0, 0), concluded the
169+
# 0.25.0 context was NEWER than the 1.0.0 install, and suppressed the nudge — on the
170+
# single upgrade the whole feature exists to catch.
171+
msg = agent_context_staleness(
172+
{"version": 1, "servers": ["python"], "clients": [], "files": {"a": "h"}, "generatedBy": "0.25.0"},
173+
"1.0.0",
174+
)
175+
assert msg is not None
176+
assert "0.25.0" in msg
177+
178+
179+
def test_a_context_a_MAJOR_ahead_is_still_silent() -> None:
180+
# The publish-what-changed exemption is unchanged and still applies WITHIN and ACROSS
181+
# majors: the canonical scaffolder is npm, so a stamp genuinely newer than this port's
182+
# install is correct and must not nudge.
183+
assert agent_context_staleness({"generatedBy": "1.0.0"}, "0.25.0") is None
184+
185+
186+
def test_same_release_and_same_series_agree_on_finals() -> None:
187+
# The two helpers parsed the same grammar with two different regexes. They must not be
188+
# able to disagree: the ordering coordinate is the equality coordinate restricted to
189+
# FINAL releases, so a final that compares equal must also compare not-ahead either way.
190+
from metaobjects.agent_context.scaffold import (
191+
_context_is_ahead_of_install,
192+
_same_release,
193+
)
194+
195+
for v in ("0.24.5", "1.0.0", "10.2.30"):
196+
assert _same_release(v, v) is True
197+
assert _context_is_ahead_of_install(v, v) is False
198+
# and a prerelease is "not orderable" to BOTH, so it keeps nudging
199+
for v in ("1.0.0-rc.5", "1.0.0rc5", "0.24.5+abc", "0.0.0", "nope"):
200+
assert _context_is_ahead_of_install(v, "1.0.0") is False

‎server/typescript/packages/metadata/src/core/vocabulary-rewrite-yaml.ts‎

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
type RetiredEntry,
4343
} from "../retired-vocabulary.js";
4444
import type { RewriteChange, RewriteRefusal, RewriteOpts, RewriteResult } from "../vocabulary-rewrite.js";
45+
import { authoredBaseRefusal } from "../vocabulary-rewrite.js";
4546

4647
/**
4748
* A rewrite result that can also report "I could not read this file".
@@ -304,24 +305,13 @@ export function rewriteYamlDocument(source: string, opts: RewriteOpts = {}): Yam
304305
if (`${entry.type}.${entry.subType}` !== key) continue;
305306
refusals.push({ ...note(entry), subject: key, line: lineOf(span.keyStart) });
306307
}
307-
// The SAME rule as the JSON arm: an authored `<type>.base` is a load error, not a
308-
// retirement, and `meta upgrade` must not answer "nothing to rewrite" about it. Kept
309-
// beside its sibling rather than in one shared pass because these two rewriters walk
310-
// different structures — the shared thing is `RewriteOpts.abstractAnchorTypes`, which
311-
// both read, so neither can be given a different answer to the same question.
312-
const dot = key.lastIndexOf(".");
313-
if (dot >= 0
314-
&& key.slice(dot + 1) === "base"
315-
&& (opts.abstractAnchorTypes ?? []).includes(key.slice(0, dot))) {
316-
refusals.push({
317-
since: "1.0.0",
318-
why:
319-
`"${key}" may not be authored — every "base" subtype is an abstract registry ` +
320-
"anchor that concrete subtypes inherit from, with no runtime semantics of its own.",
321-
migration: "docs/features/migrations/base-subtypes-are-not-authorable.md",
322-
subject: key,
323-
line: lineOf(span.keyStart),
324-
});
308+
// The SAME verdict as the JSON arm, from the SAME function. The two rewriters walk
309+
// different structures — JSON key ranges vs YAML node spans — so the DETECTION stays
310+
// per-arm, but the refusal an adopter reads is one decision and now has one source.
311+
// Only the line number is this arm's own.
312+
const baseRefusal = authoredBaseRefusal(key, opts.abstractAnchorTypes);
313+
if (baseRefusal !== undefined) {
314+
refusals.push({ ...baseRefusal, line: lineOf(span.keyStart) });
325315
}
326316
return;
327317
}

0 commit comments

Comments
 (0)