Skip to content

Commit 53cd936

Browse files
dmealingclaude
andcommitted
refactor: the simplifier pass, and two things it caught that the review did not
Four cleanup agents over this session's twelve commits. Two of their findings were correctness, not tidiness, and both were mine: **The ejectable copy of the names generator still hardcoded `fragment: true`.** `src/reference/names.ts` is what `meta init` copies into an adopter's repo, and it is otherwise character-for-character the generator I had just fixed. Its byte-identical gate could not see the divergence — that gate runs `runGen` unscoped, so pass 1 always emits the TPH base and pass 2 breaks on `emitted.has(key)` before the flag is read. The bug is reachable only under `meta gen --entities <Subtype>`, which the gate never exercises. Fixed at the depth that makes it unreachable rather than by mirroring one line: `renderNamesDecl` now DERIVES the shape from the object, so `fragment` says only "this is an ancestor render". Every `fragment: true` already on disk in an adopter's ejected copy becomes correct with no edit, and no caller can get it wrong again. Mirroring the line would also have forced a VALUE import from `@metaobjectsdev/metadata` into an ejected file, which is the dual-physical-copy hazard CLAUDE.md warns about. **I shipped a false claim while fixing a false claim.** The JSDoc I corrected two commits ago says "htmlType, placeholder and helpText are NOT emitted". `htmlType` IS emitted — derived from the view subtype; only the `@htmlType` attribute OVERRIDE was removed. The golden snapshots updated in that same commit carry `htmlType: "number"` fifteen lines below the claim. Corrected, and it says which of the two mechanisms went away. The rest is tidying, each agreed by at least two agents: - `servedPath(entity, apiPrefix)` in `api-surface.ts` replaces three open-coded copies of `${apiPrefix}${restPath(obj)}`. Three doors composing one address is the defect class these commits keep fixing; leaving a third copy behind while fixing the second was not defensible. - The new C# `Escape` was a verbatim copy of `Fr010FieldMapping.CSharpStringLiteral`, the sixth escaper in that assembly. Deleted; the allocation-free fast path moved INTO the shared helper, so its three existing callers get it too. - `declaresNamesContent` dropped a dead `typeof obj.ownFields !== "function"` duck-type check; `namesArtifactSuperOf` narrows with the exported `isMetaObject` guard instead of `as MetaObject`, which is what CLAUDE.md requires. - The effective-children `claimed` set became a flag array in TS and Python, mirroring the C# port: the length is known, this is the memoized path walked for every node, and it drops a hash lookup per element plus a closure/generator frame per own child. - A dead `resourcePath` import, a recomputed `memberDepth`, and a `"source"` literal with a double cast where `isMetaSource` narrows. Skipped, deliberately: de-duplicating the test harnesses in four files (three agents flagged it; it is churn across passing tests), and folding `ApiModelContext`'s positional threading into a resolved struct (a wider refactor than this pass warrants). Both are recorded here rather than argued with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UcDcQmAVYzYjkVGVj23mt4
1 parent 663924c commit 53cd936

41 files changed

Lines changed: 335 additions & 269 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎examples/showcase/.metaobjects/.gen-state/.hashes.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"__init__.py": "daa04cec586768866d0996c779f62073e6773fe63750ff668327991fac2fd0f5",
1111
"generated/ts/Subscriber.queries.ts": "843e21d8ed84ecead64870b980dfa3156dd2f360ade1f1d2693a5f04bde0fb3c",
1212
"generated/ts/Subscriber.routes.ts": "05d57884708ed78bdad9d69744b1685bc7a310a988aec2edd2830b53b68b3315",
13-
"generated/ts/Subscriber.ts": "abfb328f5eacc3d1c1ea890267dc7af8dfe2656592e33d032df14c76b3ca3e19",
13+
"generated/ts/Subscriber.ts": "b215a36de8a7eb3845b6b71daf72e2d4f54897441f25dc96d9fe0181ad758d87",
1414
"generated/ts/SubscriberBlurbPayload.ts": "613a80a6c0ce7e967f84f7af3c587a3c6f04434a67cea838344217ab6d132fb9",
1515
"generated/ts/index.ts": "b9f04c67786b5eec1aa493958d9843b86a58406228fcfc85634559fc4b2cbfa6",
1616
"generated/ts/prompts.ts": "9cd69070c5b9c1f4b1200422858ef34488c2e8aa8ea5339714629a8de72c81cb",

‎examples/showcase/generated/ts/Subscriber.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,16 @@ export const SubscriberInsertPreservingSchema = z.object({
6161
*
6262
* Use these instead of magic strings so TS catches typos and refactors stay
6363
* coherent. Each non-dollar-prefixed key is a per-field object carrying
64-
* name, label, view, and the RHF-shaped validation rules derived from the
65-
* field's validator children.
64+
* name, label, view, an htmlType where the field's view maps to one, and the
65+
* RHF-shaped validation rules derived from the field's validator children.
6666
*
67-
* htmlType, placeholder and helpText are NOT emitted — no provider registers
68-
* an attribute for them, so nothing here could derive one. useEntityForm
69-
* still honours all three if you add them: this file is generated, hand edits
70-
* inside it survive regeneration through the three-way merge, and the
71-
* consumers read this const rather than the metadata. That is the intended
72-
* way to set them.
67+
* placeholder and helpText are NOT emitted — no provider registers an attribute
68+
* for them, so nothing here could derive one. (htmlType is emitted, but derived
69+
* from the view subtype; the @htmlType override was removed for the same reason.)
70+
* useEntityForm still honours placeholder if you add it: this file is generated,
71+
* hand edits inside it survive regeneration through the three-way merge, and the
72+
* consumers read this const rather than the metadata. That is the intended way
73+
* to set it.
7374
*
7475
* Typical usage with the metaobjects React form helper:
7576
*

‎examples/showcase/site-payload.json‎

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.

‎server/csharp/MetaObjects.Codegen/Generators/Fr010FieldMapping.cs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,19 @@ public static bool HasXmlText(MetaData field) =>
214214
(field.Attr(MetaObjects.Template.TemplateConstants.FIELD_ATTR_XML_TEXT) is string s &&
215215
s.Equals("true", StringComparison.OrdinalIgnoreCase));
216216

217-
/// <summary>Escape a value for embedding inside a C# double-quoted string literal.</summary>
218-
public static string CSharpStringLiteral(string value)
217+
private static readonly char[] CSharpEscapeChars = { '\\', '"', '\t', '\n', '\r' };
218+
219+
/// <summary>
220+
/// Escape a value for embedding inside a C# double-quoted string literal.
221+
/// <para>Scans before it builds: the names generator calls this two-to-four times per
222+
/// member of every entity, and virtually every value — "object", "entity", a field
223+
/// name, a snake_case column — contains none of these. <c>IndexOfAny</c> is vectorized
224+
/// and allocates nothing, so the common case costs no allocation at all.</para>
225+
/// </summary>
226+
public static string CSharpStringLiteral(string value) =>
227+
value.IndexOfAny(CSharpEscapeChars) < 0 ? value : CSharpStringLiteralSlow(value);
228+
229+
private static string CSharpStringLiteralSlow(string value)
219230
{
220231
var sb = new System.Text.StringBuilder(value.Length + 4);
221232
foreach (char c in value)

‎server/csharp/MetaObjects.Codegen/Generators/NamesGenerator.cs‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -102,26 +102,7 @@ protected override EmittedFile GenerateOne(MetaObject entity, GenContext ctx) =>
102102
/// identifier and may hold either character.
103103
/// </summary>
104104
private static Member Str(string name, string value, string nodePath) =>
105-
new(name, $"const string {name} = \"{Escape(value)}\";", nodePath);
106-
107-
/// <summary>C# string-literal escaping for an author-supplied value.</summary>
108-
private static string Escape(string value)
109-
{
110-
var sb = new System.Text.StringBuilder(value.Length + 8);
111-
foreach (var c in value)
112-
{
113-
switch (c)
114-
{
115-
case '\\': sb.Append("\\\\"); break;
116-
case '"': sb.Append("\\\""); break;
117-
case '\n': sb.Append("\\n"); break;
118-
case '\r': sb.Append("\\r"); break;
119-
case '\t': sb.Append("\\t"); break;
120-
default: sb.Append(c); break;
121-
}
122-
}
123-
return sb.ToString();
124-
}
105+
new(name, $"const string {name} = \"{Fr010FieldMapping.CSharpStringLiteral(value)}\";", nodePath);
125106

126107
/// <summary>
127108
/// Every constant an artifact declares, in emission order, grouped so the emitter can

‎server/csharp/MetaObjects/Meta/MetaData.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,10 @@ private List<MetaData> EffectiveChildren(HashSet<MetaData> visited)
511511
// child is deferred so it cannot be matched later — and this closes the branch it
512512
// left open. The shape is a write-through entity (two unnamed source.rdb children)
513513
// whose base ALSO declares a source: its own primary was silently dropped.
514-
var claimed = new HashSet<int>();
514+
// A flag array, not a HashSet: `result` does not grow until after this loop, so the
515+
// length is known, and this is the memoized effective-children path walked for every
516+
// node. One allocation, index access instead of hashing, no boxing.
517+
var claimed = new bool[result.Count];
515518
foreach (var ownChild in _children)
516519
{
517520
// Find the index in result that has the same (type, name). An explicit loop
@@ -520,7 +523,7 @@ private List<MetaData> EffectiveChildren(HashSet<MetaData> visited)
520523
int idx = -1;
521524
for (int i = 0; i < result.Count; i++)
522525
{
523-
if (!claimed.Contains(i) && result[i].Type == ownChild.Type
526+
if (!claimed[i] && result[i].Type == ownChild.Type
524527
&& result[i].Name == ownChild.Name)
525528
{
526529
idx = i;
@@ -531,7 +534,7 @@ private List<MetaData> EffectiveChildren(HashSet<MetaData> visited)
531534
{
532535
// Replace the super child with our own (in-place override).
533536
result[idx] = ownChild;
534-
claimed.Add(idx);
537+
claimed[idx] = true;
535538
}
536539
else
537540
{

‎server/python/src/metaobjects/codegen/generators/names_generator.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@
9696
strip_package,
9797
to_snake_case,
9898
)
99-
from metaobjects.shared.base_types import TYPE_IDENTITY, TYPE_INDEX
99+
from metaobjects.shared.base_types import TYPE_FIELD, TYPE_IDENTITY, TYPE_INDEX
100100
from metaobjects.source_resolution import primary_rdb_source
101101

102102
#: The nodes whose DATABASE index name this module carries.
@@ -238,6 +238,12 @@ def _field_consts(field: MetaData, column: str) -> list[_Const]:
238238
]
239239

240240

241+
#: The node types a names module carries. A frozenset of module-level constants rather
242+
#: than an inline tuple: an inline one is rebuilt on every element compared, because
243+
#: CPython cannot fold a tuple of globals.
244+
_NAMES_CONTENT_TYPES = frozenset({TYPE_FIELD, TYPE_IDENTITY, TYPE_INDEX})
245+
246+
241247
def declares_names_content(entity: MetaObject) -> bool:
242248
"""Whether ``entity`` DECLARES anything a names module carries.
243249
@@ -254,10 +260,7 @@ def declares_names_content(entity: MetaObject) -> bool:
254260
declares, not what it can see. An inherited key belongs to the ancestor that
255261
declared it and is reached through that ancestor's module.
256262
"""
257-
return bool(
258-
entity.own_fields()
259-
or [c for c in entity.own_children() if c.type in (TYPE_IDENTITY, TYPE_INDEX)]
260-
)
263+
return any(c.type in _NAMES_CONTENT_TYPES for c in entity.own_children())
261264

262265

263266
def names_artifact_super_of(entity: MetaObject) -> MetaObject | None:

‎server/python/src/metaobjects/meta/meta_data.py‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -242,18 +242,21 @@ def _effective_children_inner(
242242
# branch it left open: the same write-through shape, but with the BASE also
243243
# declaring a source, so the first own source takes the replace branch.
244244
append_queue: list[MetaData] = []
245-
claimed: set[int] = set()
245+
# A flag array, not a set: `result` does not grow until after this loop, so
246+
# the length is known, and this is the memoized effective-children path walked
247+
# for every node. Also drops the per-own-child generator frame `next()` built.
248+
claimed = bytearray(len(result))
246249
for own in self._children:
247-
idx = next(
248-
(i for i, c in enumerate(result)
249-
if i not in claimed and c.type == own.type and c.name == own.name),
250-
None,
251-
)
250+
idx = None
251+
for i, c in enumerate(result):
252+
if not claimed[i] and c.type == own.type and c.name == own.name:
253+
idx = i
254+
break
252255
if idx is None:
253256
append_queue.append(own)
254257
else:
255258
result[idx] = own
256-
claimed.add(idx)
259+
claimed[idx] = 1
257260
result.extend(append_queue)
258261
return result
259262

‎server/typescript/packages/codegen-ts/src/api-surface.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import type { MetaObject } from "@metaobjectsdev/metadata";
2222
import { isAbstract } from "./instance-artifacts.js";
2323
import { isProjection } from "./projection/projection-detector.js";
2424
import { hasAnyRdbSource, hasWritableRdbSource } from "./source-detect.js";
25-
import { resourcePath } from "./templates/entity-ui-descriptor.js";
25+
import { resourcePath, restPath } from "./templates/entity-ui-descriptor.js";
2626
import {
2727
declaresTphDiscriminator,
2828
isTphSubtype,
@@ -59,6 +59,21 @@ export function servesWriteApi(entity: MetaObject): boolean {
5959
// have to care that the composition moved.
6060
export { restPath } from "./templates/entity-ui-descriptor.js";
6161

62+
/**
63+
* The address the generated routes actually SERVE an object at: the mount prefix plus the
64+
* object's REST path.
65+
*
66+
* One function, because getting this wrong is the defect it exists to prevent, twice over.
67+
* `agent/ui.md` documented `/authors` for routes mounted at `/api/authors`; the API
68+
* reference did the same, separately, in two more places — three doors composing one
69+
* address, each free to drift from the others. `templates/routes-file-hono.ts` and
70+
* `routes-file.ts` compose the same thing into GENERATED code and remain the runtime
71+
* authority; this is the documentation side mirroring them.
72+
*/
73+
export function servedPath(entity: MetaObject, apiPrefix: string): string {
74+
return `${apiPrefix}${restPath(entity)}`;
75+
}
76+
6277
/**
6378
* True when a FORM is generated for this object.
6479
*

‎server/typescript/packages/codegen-ts/src/generators/agent-ui-page.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {
4343
} from "@metaobjectsdev/metadata";
4444
import type { MetaData, MetaObject, MetaRoot } from "@metaobjectsdev/metadata";
4545
import { GENERATED_HEADER } from "../constants.js";
46-
import { hasGeneratedForm, restPath, servesReadApi } from "../api-surface.js";
46+
import { hasGeneratedForm, restPath, servedPath, servesReadApi } from "../api-surface.js";
4747
import {
4848
buildEntityUiDescriptor,
4949
type UiFieldDescriptor,
@@ -121,7 +121,7 @@ function flag(value: unknown): string {
121121
* is deliberately no form"; asking the endpoint question announced one for every base.
122122
*/
123123
function endpointLine(obj: MetaObject, root: MetaRoot, apiPrefix: string): string {
124-
const endpoint = `${apiPrefix}${restPath(obj)}`;
124+
const endpoint = servedPath(obj, apiPrefix);
125125
if (hasGeneratedForm(obj)) return `Endpoint \`${endpoint}\`.`;
126126
// `isTphDiscriminatorBase`, which requires at least one CONCRETE subtype — the same
127127
// predicate `routes-file.ts` switches on. `@discriminator` with no subtype yet is a

0 commit comments

Comments
 (0)