Skip to content

Commit f403354

Browse files
dmealingclaude
andcommitted
fix(cli): meta upgrade printed one rationale per occurrence, not per rule
An estate carrying 24 `@forge*` attributes got ~96 lines for ONE retirement: each occurrence repeated the same why, the same two options and the same guide link. The instruction was buried in its own repetition — the failure runGen's no-manifest refusal aggregates to avoid, in a command that had not learned it. Occurrences stay per line, because file/line/subject/value is per-line information. The rationale is per rule and prints once, after the listing, headed by the subjects it covers — without that heading a paragraph printed away from its lines leaves the reader guessing which refusals it belongs to. Keyed by the rendered text, so two rules that read identically collapse and two that differ anywhere do not. Verified against the estate's own metadata: that file went from 24 lines to 9. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent 4bfeb9e commit f403354

3 files changed

Lines changed: 115 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
77

88
## [Unreleased]
99

10+
### Fixed — `meta upgrade` printed one retirement's rationale once per OCCURRENCE
11+
12+
An adopter estate carrying 24 `@forge*` attributes got ~96 lines of output for one
13+
retirement: every occurrence repeated the same `why`, the same two options and the same
14+
guide link. The instruction was buried in its own repetition — the exact failure
15+
`runGen`'s no-manifest refusal aggregates to avoid, in a command that had not learned it.
16+
17+
Occurrences are per-line information and still print per line (file, line, subject, the
18+
value found). The rationale is per RULE and now prints once, after the listing, headed by
19+
the attributes it covers so a reader can connect the paragraph to the lines above it. Two
20+
rules that render identically collapse; two that differ anywhere do not.
21+
22+
1023
### Fixed — a failed D1 introspection named a WARNING as its cause
1124

1225
`meta verify --db` / `meta migrate` against a live D1 shell out to

‎server/typescript/packages/cli/src/commands/upgrade.ts‎

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ export async function upgradeCommand(args: string[], cwd: string): Promise<numbe
126126
...(flags.maxVersion !== undefined ? { maxVersion: flags.maxVersion } : {}),
127127
};
128128

129+
// One retirement RULE, however many occurrences it has. An estate hit this with 24
130+
// `@forge*` attributes, and the per-occurrence rationale turned the output into ~96
131+
// lines saying one thing four times over — the wall that buries the instruction it is
132+
// made of, which is the exact failure `runGen`'s no-manifest refusal aggregates to
133+
// avoid. Keyed by the rendered text, so two rules that happen to read identically
134+
// collapse and two that differ anywhere do not.
135+
const rules = new Map<string, { why: string; suggestions: string[]; subjects: Set<string> }>();
136+
129137
for (const file of files) {
130138
const rel = relative(projectRoot, file);
131139
const before = await readFile(file, "utf8");
@@ -148,22 +156,24 @@ export async function upgradeCommand(args: string[], cwd: string): Promise<numbe
148156
log.info(`\n${rel}`);
149157
for (const c of r.changes) log.info(` ${c.line}: @${c.from} → ${c.to}`);
150158
for (const f of r.refusals) {
151-
// The WHY and the replacement both print, and the guide is not an alternative to
152-
// them. This used to read `Retired in <since>. See <guide>` — dropping `why`
153-
// whenever a guide existed, which is almost always, and dropping `replacedBy`
154-
// unconditionally. For the `@forge*` entries that removed the only actionable
155-
// sentence there is ("opt the provider in explicitly"), and for an authored
156-
// `<type>.base` it announced a retirement of something that was never authorable —
157-
// telling the reader they used to have a working feature. Hence "no longer accepted
158-
// as of", which is true of both, with the carefully-written `why` beside it.
159+
// The occurrence line is per-line information: file, line, subject, the value found.
160+
// The RATIONALE that goes with it is per RULE and prints once, after the listing —
161+
// see `rules` below.
159162
log.warn(
160163
` ${f.line}: ${f.subject}${f.value !== undefined ? `: ${f.value}` : ""} — needs a decision ` +
161164
`(no longer accepted as of ${f.since}).`,
162165
);
163-
log.warn(` ${f.why}`);
164-
// The same suggestion list every OTHER door prints for a retirement, rather than a
165-
// second rendering of the same note that can drift from it.
166-
for (const line of retirementSuggestions(f)) log.warn(` ${line}`);
166+
const key = `${f.why}\u0000${retirementSuggestions(f).join("\u0000")}`;
167+
const rule = rules.get(key);
168+
if (rule === undefined) {
169+
rules.set(key, {
170+
why: f.why,
171+
suggestions: retirementSuggestions(f),
172+
subjects: new Set([f.subject]),
173+
});
174+
} else {
175+
rule.subjects.add(f.subject);
176+
}
167177
}
168178

169179
totalChanges += r.changes.length;
@@ -175,6 +185,21 @@ export async function upgradeCommand(args: string[], cwd: string): Promise<numbe
175185
}
176186

177187
log.info("");
188+
189+
// The WHY and the options, once per rule. Both print, and the guide is not an
190+
// alternative to them: this used to read `Retired in <since>. See <guide>`, dropping
191+
// `why` whenever a guide existed — which is almost always — and dropping `replacedBy`
192+
// unconditionally. For the `@forge*` entries that removed the only actionable sentence
193+
// there is ("opt the provider in explicitly"). The subject list is what lets a reader
194+
// connect the paragraph back to the lines above it now that it prints away from them.
195+
for (const rule of rules.values()) {
196+
const subjects = [...rule.subjects].sort().join(", ");
197+
log.warn(`\n ${subjects} — why, and what to do:`);
198+
log.warn(` ${rule.why}`);
199+
for (const line of rule.suggestions) log.warn(` ${line}`);
200+
}
201+
if (rules.size > 0) log.info("");
202+
178203
if (notChecked.length > 0) {
179204
log.warn(
180205
`${notChecked.length} file(s) could not be parsed and were NOT checked — fix these ` +

‎server/typescript/packages/cli/test/forge-retirement-named.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,68 @@ describe("meta upgrade and the deregistered @forge* vocabulary", () => {
8686
expect(err).toContain("@forgeRationale"); // on field.long
8787
});
8888
});
89+
90+
// ── the wall ──────────────────────────────────────────────────────────────────
91+
//
92+
// An adopter estate hit this with 24 `@forge*` occurrences. Every one printed the same
93+
// three lines — the `why`, the keep-them option, the delete-them option, the guide — so
94+
// the output was ~96 lines saying one thing four times over, and the instruction was
95+
// buried in its own repetition. That is exactly the failure `runGen`'s no-manifest
96+
// refusal aggregates to avoid, in a command that had not learned it.
97+
//
98+
// Occurrences are per-line information and stay per line. The RATIONALE is per RULE, and
99+
// prints once.
100+
describe("many occurrences of one retirement", () => {
101+
const MANY = {
102+
"metadata.root": {
103+
package: "acme",
104+
children: ["A", "B", "C", "D"].map((n) => ({
105+
"object.entity": {
106+
name: n, "@forgeConfidence": "high", "@forgeSource": "an interview",
107+
children: [
108+
{ "source.rdb": { "@table": n.toLowerCase() } },
109+
{ "field.long": { name: "id" } },
110+
{ "identity.primary": { "@fields": ["id"], "@generation": "increment" } },
111+
],
112+
},
113+
})),
114+
},
115+
};
116+
117+
async function upgradeMany(): Promise<{ exit: number; err: string }> {
118+
const dir = await mkdtemp(join(tmpdir(), "forge-wall-"));
119+
dirs.push(dir);
120+
await mkdir(join(dir, "metaobjects"), { recursive: true });
121+
await writeFile(join(dir, "metaobjects", "meta.json"), JSON.stringify(MANY), "utf8");
122+
const lines: string[] = [];
123+
const oe = console.error, ol = console.log;
124+
console.error = (...a: unknown[]) => { lines.push(a.map(String).join(" ")); };
125+
console.log = (...a: unknown[]) => { lines.push(a.map(String).join(" ")); };
126+
try { return { exit: await run(["upgrade", "--cwd", dir]), err: lines.join("\n") }; }
127+
finally { console.error = oe; console.log = ol; }
128+
}
129+
130+
test("every occurrence is still named", async () => {
131+
const { exit, err } = await upgradeMany();
132+
expect(exit).toBe(1);
133+
// 4 entities × 2 attributes = 8 occurrence lines.
134+
expect(err.split("\n").filter((l) => l.includes("needs a decision"))).toHaveLength(8);
135+
});
136+
137+
test("the reason and the options print ONCE, not once per occurrence", async () => {
138+
const { err } = await upgradeMany();
139+
const count = (needle: string): number => err.split("\n").filter((l) => l.includes(needle)).length;
140+
expect(count("expected-registry.json")).toBe(1); // the why
141+
expect(count("forgeTypesProvider")).toBe(1); // keep them
142+
expect(count("0.x-to-1.0.md")).toBe(1); // the guide
143+
});
144+
145+
test("the rationale names which attributes it is about", async () => {
146+
// Printed away from the occurrence lines, it has to say what it applies to — or the
147+
// reader has to guess which of the refusals the paragraph belongs to.
148+
const { err } = await upgradeMany();
149+
const idx = err.indexOf("expected-registry.json");
150+
const paragraph = err.slice(Math.max(0, idx - 400), idx);
151+
expect(paragraph).toContain("@forgeConfidence");
152+
});
153+
});

0 commit comments

Comments
 (0)