Skip to content

Commit 865c1ef

Browse files
dmealingclaude
andcommitted
fix(python): key .gen-state by the project, not by --out
The manifest lives at the project root — `<project>/.metaobjects/.gen-state`, anchored on the metadata directory's parent, deliberately and matching C#. Its KEYS were relative to `--out`. So two runs with different `--out` shared ONE manifest keyed by names that mean different files: `gen --out a` and `gen --out b` both record `Program.py`, and whichever ran last decided whether the other run's file still counted as ours. That manifest is the only thing that can tell "this is exactly what I wrote" from "somebody edited this" on a machine that did not generate the code. One that can be populated from an unrelated tree and keyed ambiguously is one that can say yes to the wrong file. The multi-target path (#267) has the same shape by construction: every target writes its own outDir and they all share one gen-state dir. TypeScript keys project-root-relative, which is what makes multiple targets work there. Keys are now relative to the project. The old spelling is read as a FALLBACK and dropped once each file is recorded under the new key, so an existing manifest keeps working and converges with no migration command — re-keying with no fallback is fail-closed (`gen` REFUSES a now-unrecorded file) and the wrong cost to impose on adopters for a change they did not ask for. `verify --codegen`'s jurisdiction guard reads the same manifest and had to move with it. That half fails in the direction nothing notices: a lookup that finds nothing makes every file "not ours", so the `extra` verdict silently empties while the gate still prints `in sync`. Reproduced exactly — the break-probe run printed "in sync (0 file(s))" over stale committed output. The report that found this also claimed `_record` replaces the manifest wholesale (21 entries → 6). Not reproducible from the code: `_record` loads and adds. Only the keying was wrong. Gates: the keying and the legacy fallback as units; the key shape, the two-out- dir case and the `extra` conviction end to end through the CLI, because the keying is decided in three places and a unit test of any one passes while the other two disagree. `test_verify_codegen_ignores_a_file_it_never_wrote` covered only the negative direction and would have stayed green throughout. Migration guide §13. No metadata contract changes; `metamodelVersion` unmoved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent c90feef commit 865c1ef

7 files changed

Lines changed: 372 additions & 31 deletions

File tree

‎docs/features/migrations/0.x-to-1.0.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,47 @@ longer registered by default**. Metadata carrying any of them fails the load wit
363363
`writeRecord`, `listRecords`, `promoteRecord`, `supersede`, …). Nothing in the library
364364
and no adopter estate imported them.
365365

366+
## 13. Two tool-state defaults that move at the cut
367+
368+
Neither is metadata — no `metamodelVersion` move, no vocabulary change — but both are
369+
adopter-visible, so they are here rather than only in the changelog.
370+
371+
### `meta docs` now defaults `outDir` to `./docs/generated`
372+
373+
It was `./docs`, which in most repositories is the *human* documentation folder. A bare
374+
`meta docs` scattered 29 generated pages through one estate's `docs/` — a business-strategy
375+
document, product screenshots, email drafts — and wanted to own `docs/README.md`. The docs
376+
drift gate's own rule is that `docs.outDir` is a directory, not a namespace MetaObjects
377+
owns, which is precisely the argument against defaulting into the most-owned directory name
378+
in the ecosystem.
379+
380+
**If you were relying on the old default**, pin it explicitly — one line, and your pages
381+
stay exactly where they are:
382+
383+
```ts
384+
export default defineConfig({
385+
docs: { outDir: "./docs" },
386+
});
387+
```
388+
389+
Otherwise the next `meta docs` writes to `docs/generated/` and `meta verify --docs`
390+
measures there. `meta init` now scaffolds the key, so a new project's choice is visible
391+
rather than implied. (`--out` is still a per-run override; it does not persist, which is
392+
why the config key is what a gate needs.)
393+
394+
### Python: `.gen-state/.hashes.json` keys are project-relative
395+
396+
The manifest lives at the project root, and its keys were relative to `--out`. Two runs
397+
with different `--out` therefore shared one manifest keyed by names that mean different
398+
files, and the manifest is the only thing that can tell "this is exactly what I wrote"
399+
from "somebody edited this" on a machine that did not generate the code. The multi-target
400+
path (`metaobjects.config.yaml`) had the same shape by construction. Keys are now relative
401+
to the project, as the TypeScript port's have always been.
402+
403+
**Nothing to do.** The old spelling is read as a fallback and dropped the next time each
404+
file is recorded, so an existing manifest keeps working and converges on its own. The
405+
first `metaobjects gen` after upgrading will rewrite the keys; commit the result.
406+
366407
## Additive in 1.0 (no migration required)
367408

368409
- **`field.uri` / `field.inet` subtypes + `@stringFormat`** (`email` / `hostname`)

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

Lines changed: 68 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import argparse
4545
import dataclasses
4646
import json
47+
import os
4748
import re
4849
import sys
4950
import tempfile
@@ -376,6 +377,7 @@ def _run_suite(
376377
emit_package_init: bool = True,
377378
gen_state_dir: str | None = None,
378379
column_naming: str = DEFAULT_COLUMN_NAMING,
380+
project_root: str | None = None,
379381
) -> list[str]:
380382
"""Run a generator suite against an ALREADY-LOADED ``root`` into ``out_dir``.
381383
@@ -405,6 +407,11 @@ def _run_suite(
405407
emit_package_init=emit_package_init,
406408
gen_state_dir=gen_state_dir,
407409
column_naming=column_naming,
410+
# Manifest keys are relative to the PROJECT, which is what the manifest is
411+
# anchored on — not to ``--out``, which varies per run while the manifest does
412+
# not. Passed alongside gen_state_dir because the two are one decision: state
413+
# that lives in the project must be keyed by the project.
414+
project_root=project_root,
408415
)
409416
suite = generators if generators is not None else _default_generators()
410417
result = run_gen(config, root, generators=suite, entity_filter=entity_filter)
@@ -792,10 +799,14 @@ def _cmd_gen(args: argparse.Namespace) -> int:
792799
print(f" {msg}", file=sys.stderr)
793800
return 1
794801
gen_state = gen_state_dir_for(args.metadata_dir)
802+
# Same anchor the manifest itself uses. Both passes below key against it, so a
803+
# `--template-spec` artifact and a default-suite artifact land in one key space.
804+
gen_project_root = str(project_root_for(args.metadata_dir))
795805
column_naming = getattr(args, "column_naming", None) or DEFAULT_COLUMN_NAMING
796806
written = _run_suite(
797807
root, args.out, generators, entities,
798808
gen_state_dir=gen_state, column_naming=column_naming,
809+
project_root=gen_project_root,
799810
)
800811
if spec_gens:
801812
# The template-spec pass renders user-supplied templates: a bad ref or a
@@ -807,6 +818,7 @@ def _cmd_gen(args: argparse.Namespace) -> int:
807818
spec_written = _run_suite(
808819
root, args.out, spec_gens, entities, emit_package_init=False,
809820
gen_state_dir=gen_state, column_naming=column_naming,
821+
project_root=gen_project_root,
810822
)
811823
except RenderError as exc:
812824
print(
@@ -828,9 +840,15 @@ def _run_gen_targets(
828840
root: MetaData,
829841
*,
830842
gen_state_dir: str | None = None,
843+
project_root: str | None = None,
831844
) -> tuple[list[str], list[str]]:
832845
"""Run each target's suite into its ``outDir``. Returns (all_written, errors).
833846
847+
This is the path the manifest keying matters most on: every target writes into its
848+
OWN ``outDir`` while all of them share ONE ``gen_state_dir``, so out-dir-relative
849+
keys meant target B's hash could claim ownership of target A's identically-named
850+
file. ``project_root`` is the common anchor that makes the keys distinct.
851+
834852
Cross-target duplicate-output-path guard (#267): ``run_gen``'s collision guard
835853
is per-pass, so two targets writing the same full path would silently clobber
836854
(generated files carry the @generated header). We accumulate every written full
@@ -851,7 +869,8 @@ def _run_gen_targets(
851869
out_dir = config.out_dir_for(t)
852870
try:
853871
written = _run_suite(
854-
root, out_dir, gens, t.entities, gen_state_dir=gen_state_dir
872+
root, out_dir, gens, t.entities, gen_state_dir=gen_state_dir,
873+
project_root=project_root,
855874
)
856875
except ValueError as exc: # intra-target run_gen collision → clean error
857876
errors.append(f"target '{t.name}': {exc}")
@@ -921,6 +940,7 @@ def _cmd_gen_neutral_fallback(args: argparse.Namespace) -> int:
921940
written = _run_suite(
922941
root, args.out, generators, entities,
923942
gen_state_dir=gen_state, column_naming=column_naming,
943+
project_root=str(root_dir.resolve()),
924944
)
925945
for path in written:
926946
print(path)
@@ -982,7 +1002,9 @@ def _cmd_gen_config(args: argparse.Namespace) -> int:
9821002
return 1
9831003

9841004
written, errors = _run_gen_targets(
985-
config, targets, root, gen_state_dir=gen_state_dir_for(config.metadata_dir())
1005+
config, targets, root,
1006+
gen_state_dir=gen_state_dir_for(config.metadata_dir()),
1007+
project_root=str(project_root_for(config.metadata_dir())),
9861008
)
9871009
if errors:
9881010
for msg in errors:
@@ -1033,28 +1055,44 @@ def _relative_set(root: Path) -> dict[str, str]:
10331055
return files
10341056

10351057

1036-
def _is_ours_for(gen_state_dir: str | None):
1058+
def _is_ours_for(gen_state_dir: str | None, out_prefix: str = ""):
10371059
"""Jurisdiction predicate for the ``extra`` verdict: did WE write this path?
10381060
10391061
``out_dir`` is a DIRECTORY, not a namespace this tool owns — the same ruling the
10401062
TypeScript gate reached (0.24.3), for the same reason. Broadening the comparison
10411063
beyond ``*.py`` means the gate now sees every stranger's file too, and convicting
10421064
those turns a project with zero drift into a red build.
10431065
1044-
The manifest already records what we WROTE, keyed relative to ``out_dir`` — the
1045-
same key space this diff uses — so it answers the ownership question exactly.
1066+
The manifest already records what we WROTE, so it answers the ownership question
1067+
exactly — but it is keyed relative to the PROJECT while this diff is keyed relative
1068+
to ``out_dir``. ``out_prefix`` (the out dir, relative to the project) is what turns
1069+
one into the other. Getting this wrong is silent in the direction that matters: a
1070+
lookup that finds nothing makes ``is_ours`` false for every file, and the gate simply
1071+
stops reporting stale output while still printing a clean verdict.
1072+
1073+
The out-dir-relative name is tried as a fallback, because that is how a manifest
1074+
written before the re-keying spells it — the same fallback ``decide_and_write``
1075+
makes, so the write path and the gate cannot disagree about which entry is a record
1076+
of this file.
1077+
10461078
FAILS CLOSED, matching :func:`is_pristine_generated`: with no manifest at all
10471079
nothing is proven, so every file stays in scope and the old verdict stands.
10481080
"""
10491081
if gen_state_dir is None or not has_hash_manifest(gen_state_dir):
10501082
return lambda _rel: True
1051-
return lambda rel: read_generated_hash(gen_state_dir, rel) is not None
1083+
1084+
def is_ours(rel: str) -> bool:
1085+
key = os.path.join(out_prefix, rel) if out_prefix else rel
1086+
return read_generated_hash(gen_state_dir, key, legacy_rel_path=rel) is not None
1087+
1088+
return is_ours
10521089

10531090

10541091
def _diff_report(
10551092
expected: dict[str, str],
10561093
committed: dict[str, str],
10571094
gen_state_dir: str | None = None,
1095+
out_prefix: str = "",
10581096
) -> int:
10591097
"""Compare a regenerated file map against the committed one; print the
10601098
standard ``verify --codegen`` drift report. Returns 0 (in sync) or 1.
@@ -1065,8 +1103,11 @@ def _diff_report(
10651103
10661104
``gen_state_dir`` scopes the ``extra`` verdict to files we have a record of
10671105
writing — see :func:`_is_ours_for`. Omit it to keep the unscoped verdict.
1106+
``out_prefix`` is the committed out dir relative to the project, which is what
1107+
translates this diff's out-dir-relative keys into the manifest's project-relative
1108+
ones; omit it only when the two coincide.
10681109
"""
1069-
is_ours = _is_ours_for(gen_state_dir)
1110+
is_ours = _is_ours_for(gen_state_dir, out_prefix)
10701111
changed = sorted(
10711112
k for k in expected if k in committed and expected[k] != committed[k]
10721113
)
@@ -1179,7 +1220,14 @@ def _verify_codegen(args: argparse.Namespace) -> int:
11791220
expected = _relative_set(Path(tmp))
11801221
committed = _relative_set(Path(args.out))
11811222

1182-
return _diff_report(expected, committed, gen_state_dir_for(args.metadata_dir))
1223+
# The manifest is keyed relative to the project; this diff is keyed relative to
1224+
# `--out`. Hand over the prefix that translates one into the other, or the
1225+
# jurisdiction lookup finds nothing and the `extra` verdict silently empties.
1226+
project_root = project_root_for(args.metadata_dir)
1227+
return _diff_report(
1228+
expected, committed, gen_state_dir_for(args.metadata_dir),
1229+
os.path.relpath(Path(args.out).resolve(), project_root),
1230+
)
11831231

11841232

11851233
def _verify_codegen_neutral_fallback(args: argparse.Namespace) -> int:
@@ -1234,7 +1282,8 @@ def _verify_codegen_neutral_fallback(args: argparse.Namespace) -> int:
12341282
# `_cmd_gen_neutral_fallback` writes its manifest to), so the jurisdiction lookup
12351283
# must be built from that, not from args.metadata_dir (None here by definition).
12361284
return _diff_report(
1237-
expected, committed, str(root_dir.resolve() / ".metaobjects" / ".gen-state")
1285+
expected, committed, str(root_dir.resolve() / ".metaobjects" / ".gen-state"),
1286+
os.path.relpath(Path(args.out).resolve(), root_dir.resolve()),
12381287
)
12391288

12401289

@@ -1351,7 +1400,16 @@ def _verify_codegen_config(args: argparse.Namespace) -> int:
13511400
# Same jurisdiction rule as _diff_report — this rung inlines its diff to
13521401
# report per-target, so the guard has to be inlined with it. Without it,
13531402
# broadening the comparison past *.py convicts every stranger's file.
1354-
is_ours = _is_ours_for(gen_state_dir_for(config.metadata_dir()))
1403+
#
1404+
# PER-TARGET prefix, because that is the whole point on this rung: two
1405+
# targets with different outDirs share one manifest, and the key that tells
1406+
# their identically-named files apart is the one anchored on the project.
1407+
is_ours = _is_ours_for(
1408+
gen_state_dir_for(config.metadata_dir()),
1409+
os.path.relpath(
1410+
Path(real_outdir).resolve(), project_root_for(config.metadata_dir())
1411+
),
1412+
)
13551413
extra = sorted(k for k in committed if k not in expected and is_ours(k))
13561414

13571415
names = "+".join(

‎server/python/src/metaobjects/codegen/config.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,22 @@ class GenConfig:
1717
# ``<project>/.metaobjects/.gen-state``. COMMIT the manifest: it is the only thing
1818
# that makes hand-edit detection work on a machine that did not generate the code.
1919
gen_state_dir: str | None = None
20+
# The project the manifest belongs to — the directory holding ``metaobjects/`` and
21+
# ``.metaobjects/``. Manifest keys are relative to THIS, not to ``out_dir``.
22+
#
23+
# They used to be out-dir-relative while the manifest itself is anchored on the
24+
# project, so two runs with different ``--out`` shared one manifest keyed by names
25+
# that mean different files: `gen --out /tmp/a` and `gen --out /tmp/b` both record
26+
# `alarm_names.py`, and run B's hash then claims ownership of run A's file. The
27+
# manifest is the only thing that can tell "this is exactly what I wrote" from
28+
# "somebody edited this" on a machine that did not generate the code, so one that can
29+
# be populated from an unrelated tree and keyed ambiguously is one that can say yes to
30+
# the wrong file. TS keys project-root-relative, which is also what makes multiple
31+
# output targets work.
32+
#
33+
# None keeps the out-dir-relative keys, so a programmatic caller that never had a
34+
# project root is unchanged. The CLI always sets it.
35+
project_root: str | None = None
2036
# Only "flat" is implemented; a non-default value is REFUSED below rather than
2137
# silently ignored (see __post_init__).
2238
output_layout: str = "flat"

‎server/python/src/metaobjects/codegen/overwrite_policy.py‎

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,14 @@ def content_hash(content: str) -> str:
4646
Same algorithm as every other port, so identical file content hashes identically
4747
everywhere.
4848
49-
The KEYS, however, deliberately differ: TypeScript keys by path relative to the
50-
PROJECT ROOT (it supports multiple output targets), while this port and C# key
51-
relative to their single out dir. A manifest is therefore NOT portable between
52-
ports. An earlier version of this docstring claimed a conformance fixture could
53-
compare them directly — it cannot, and the claim was never true.
54-
55-
Because the key here is out-dir-relative, running gen twice with different out dirs
56-
against ONE ``gen_state_dir`` collides two distinct files onto one key. Give each
57-
out dir its own gen-state dir.
49+
The KEYS are relative to the PROJECT ROOT, as TypeScript's are — the manifest is
50+
anchored on the project, so a key that means "this name, relative to whichever out
51+
dir ran last" made two runs with different ``--out`` share one manifest in which run
52+
B's hash claimed ownership of run A's file. (There is no per-out-dir gen-state dir to
53+
escape into: ``gen_state_dir`` is derived from the metadata directory, so the advice
54+
this docstring used to give could not be followed.) A manifest is still NOT portable
55+
between ports — an earlier version of this docstring claimed a conformance fixture
56+
could compare them directly, and it never could.
5857
"""
5958
return hashlib.sha256(content.encode("utf-8")).hexdigest()
6059

@@ -100,23 +99,47 @@ def _save_hashes(gen_state_dir: str, hashes: dict[str, str]) -> None:
10099
fh.write("\n")
101100

102101

103-
def read_generated_hash(gen_state_dir: str, rel_path: str) -> str | None:
104-
"""The hash recorded when we last wrote ``rel_path``, or None if never."""
105-
return _load_hashes(gen_state_dir).get(rel_path)
102+
def read_generated_hash(
103+
gen_state_dir: str, rel_path: str, legacy_rel_path: str | None = None
104+
) -> str | None:
105+
"""The hash recorded when we last wrote ``rel_path``, or None if never.
106+
107+
``legacy_rel_path`` is the OUT-DIR-relative key the same file was recorded under
108+
before keys became project-root-relative. Reading both is what keeps the re-keying
109+
from invalidating every existing manifest: without it a previously-recorded file
110+
becomes unrecorded, which is fail-closed (``gen`` REFUSES it) but makes every
111+
adopter regenerate to get past a change they did not ask for. The legacy entry is
112+
dropped the next time the file is recorded, so a manifest converges on one spelling
113+
without a migration command.
114+
"""
115+
hashes = _load_hashes(gen_state_dir)
116+
recorded = hashes.get(rel_path)
117+
if recorded is None and legacy_rel_path is not None:
118+
return hashes.get(legacy_rel_path)
119+
return recorded
106120

107121

108-
def is_pristine_generated(gen_state_dir: str, rel_path: str, current: str) -> bool:
122+
def is_pristine_generated(
123+
gen_state_dir: str, rel_path: str, current: str, legacy_rel_path: str | None = None
124+
) -> bool:
109125
"""Whether the file is byte-for-byte what we recorded writing.
110126
111127
FAILS CLOSED — False when it cannot be proven.
112128
"""
113-
recorded = read_generated_hash(gen_state_dir, rel_path)
129+
recorded = read_generated_hash(gen_state_dir, rel_path, legacy_rel_path)
114130
return recorded is not None and recorded == content_hash(current)
115131

116132

117-
def _record(gen_state_dir: str, rel_path: str, content: str) -> None:
133+
def _record(
134+
gen_state_dir: str, rel_path: str, content: str, legacy_rel_path: str | None = None
135+
) -> None:
118136
hashes = _load_hashes(gen_state_dir)
119137
hashes[rel_path] = content_hash(content)
138+
# Drop the pre-re-keying spelling in the same write. Leaving it would keep a key
139+
# that means "this name, relative to whichever out dir ran last" alive forever —
140+
# which is the ambiguity being removed.
141+
if legacy_rel_path is not None and legacy_rel_path != rel_path:
142+
hashes.pop(legacy_rel_path, None)
120143
_save_hashes(gen_state_dir, hashes)
121144

122145

@@ -133,17 +156,22 @@ def decide_and_write(
133156
*,
134157
gen_state_dir: str | None = None,
135158
rel_path: str | None = None,
159+
legacy_rel_path: str | None = None,
136160
) -> str:
137161
"""Decide and perform the write for one generated file.
138162
139163
``gen_state_dir`` enables hash-based hand-edit detection; ``rel_path`` is the key
140164
it is recorded under (defaults to the basename, which is correct for a single
141-
flat output directory).
165+
flat output directory). ``legacy_rel_path`` is the key the same file may already
166+
be recorded under from before keys became project-root-relative — read as a
167+
fallback, and removed once the file is recorded under the new key.
142168
"""
143169
if not os.path.exists(path):
144170
_write(path, content)
145171
if gen_state_dir is not None:
146-
_record(gen_state_dir, rel_path or os.path.basename(path), content)
172+
_record(
173+
gen_state_dir, rel_path or os.path.basename(path), content, legacy_rel_path
174+
)
147175
return "new"
148176

149177
if strategy == "skip-existing":
@@ -165,12 +193,12 @@ def decide_and_write(
165193
if current == content:
166194
# Nothing to do, but record it: a first run over already-correct output should
167195
# leave the file recognisable as ours next time.
168-
_record(gen_state_dir, key, content)
196+
_record(gen_state_dir, key, content, legacy_rel_path)
169197
return "unchanged"
170198

171-
if is_pristine_generated(gen_state_dir, key, current):
199+
if is_pristine_generated(gen_state_dir, key, current, legacy_rel_path):
172200
_write(path, content)
173-
_record(gen_state_dir, key, content)
201+
_record(gen_state_dir, key, content, legacy_rel_path)
174202
return "overwrite"
175203

176204
# Edited, or never recorded. Deliberately does NOT record the current content:

0 commit comments

Comments
 (0)