fix(cli): the per-package de-duplication key ignores the top-level collection index, so an echo no longer survives it

.changeset/18779-per-package-dedup-positional-key.md

@@ -0,0 +1,58 @@
+---
+'@objectstack/cli': patch
+---
+
+The per-package author-time de-duplication key ignores the top-level collection index, so a package-local finding no longer survives as an echo of the union finding it duplicates
+
+`runPerPackageAuthoringRules` runs the author-time rule table once per
+`packages[]` entry and drops anything the union run already reported. Its key
+was `rule` + `where` + `path` + `message`, and `path` is **positional**: a
+package body re-bases every collection from 0, while the flattened union numbers
+that same entry wherever `authoringRuleUnionStack` placed it.
+`objects[0].fields.industry` and `objects[1].fields.industry` are ONE finding
+under two spellings, so the `Set` never matched them and the echo survived the
+filter that exists to remove it.
+
+Measured on `origin/main` a43b9d0654 over the repo's own two-package fixture
+`examples/app-multi-package`, at every door, before and after:
+
+| | before | after |
+|---|---|---|
+| `os build --json` | warnings 4, exit 0 | warnings 3, exit 0 |
+| `os validate --json` | warnings 4, exit 0 | warnings 3, exit 0 |
+| `os lint --json` | total 4, failing 0, exit 0 | total 3, failing 0, exit 0 |
+| `os lint --json --strict` | total 4, failing 4, exit 1 | total 3, failing 3, exit 1 |
+
+The one warning that stops being reported is `field-no-consumers` on
+`crm_account.industry` re-reported at the package-local index — the union run's
+own finding, printed a second time. Its twin is still reported, which is why no
+verdict moves.
+
+**No input's verdict changes, and that is structural rather than a property of
+this fixture.** Every finding the de-duplication drops has, by construction, a
+finding carrying the same key already in the reported set: the seed is the union
+run's findings, which every door reports, and it grows only with per-package
+findings that themselves survived. So a door's refusal cannot flip — `os build`
+already exits 1 on a union error before this pass runs, and `os lint --strict`
+fails on `errors + warnings`, a count that could only reach zero if the twin
+went unreported too.
+
+Only the **top-level** index is neutralised. Nested positions (`.indexes[1]`,
+`.columns[0]`) address the author's own document and read identically in both
+views, so they stay in the key and keep discriminating. A finding's own `path`
+is never modified — every door still prints the location it always printed.
+
+What this does **not** buy: the key becomes position-insensitive, not
+collision-proof. Two entries that render the same `where` still share a key,
+exactly as they already did whenever their indices happened to match. Measured
+over every example stack in this repo that parses today (`app-multi-package`'s
+built artifact, `app-crm`, `app-showcase`, `app-todo`), 45 registry rules
+produced 103 findings and 103 distinct neutralised keys — zero collisions.
+
+Also corrected: the sentence "what survives the filter is exactly the set the
+union could not see", which was false for as long as the key was positional and
+had been copied from `compile.ts` into the `os validate` and `os lint` doors as
+each was wired. It is now stated at the bound the pass can actually hold, in
+every file that carried it.
+
+Clause-②: no

packages/cli/src/commands/compile.ts

@@ -452,8 +452,22 @@ export default class Compile extends Command {
       //     DE-DUPLICATED against the union run, because the union contains
       //     every package's items: without this, a two-package project reports
       //     every finding twice and the author cannot tell a real per-package
-      //     finding from an echo. What survives the filter is exactly the set
-      //     the union could not see.
+      //     finding from an echo.
+      //
+      //     ⚠️ [#18779] This comment used to end "What survives the filter is
+      //     exactly the set the union could not see", and that was FALSE for
+      //     as long as the de-duplication key carried the POSITIONAL `path`:
+      //     a package body re-bases its collections from 0, so one finding got
+      //     two keys and its echo survived the very filter described here. The
+      //     sentence was quoted as authority by #18677 and #18778 without the
+      //     definition being opened, and copied into the `os validate` and
+      //     `os lint` doors as each was wired. `findingKey` now neutralises
+      //     the top-level collection index, so what survives is the set of
+      //     per-package findings no union finding already carried under the
+      //     same rule, `where`, message and non-top-level position. ⛔ Do not
+      //     re-inflate that to "exactly the set the union could not see" —
+      //     `utils/artifact-packages.ts` states the bound and why it is
+      //     narrower than that sentence.
       //
       //     [#16611] Each package's stack is handed the artifact's `packages[]`
       //     as RESOLUTION CONTEXT — see `packageBodyAsStack`. The list read here

packages/cli/src/commands/lint.ts

@@ -684,10 +684,19 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
   //
   // The second half of the run above, and the half THIS door ran without.
   // `os build` has run it since #16611 and `os validate` since #18677; `os
-  // lint` ran the union fold and stopped. `compile.ts` step 3b-ii says what
-  // survives the de-duplication is "exactly the set the union could not see" ⇒
-  // that whole set was findings `os build` reported and this command
-  // structurally could not.
+  // lint` ran the union fold and stopped. Every finding this pass yields is
+  // therefore one `os build` reported and this command structurally could not.
+  //
+  // ⚠️ [#18779] This paragraph used to size that gap by quoting `compile.ts`
+  // step 3b-ii — "exactly the set the union could not see" — and that sentence
+  // was FALSE when it was copied here: the de-duplication key carried the
+  // POSITIONAL `path`, so a package-local finding and its flattened twin got
+  // two keys and the ECHO survived. Part of every survivor set was therefore
+  // something this door's own union run ALREADY reported. The key was
+  // corrected in `utils/artifact-packages.ts`; the gap this door closed is
+  // real and its direction is unchanged, but ⛔ do not re-derive its size from
+  // that sentence — it was quoted, never measured, by the two cards that
+  // wired the second and third doors.
   //
   // ⚠️ The reading that hid it for two cards is the one the imports above
   // invite: this file DOES call `artifactPackages` and `packageBodyAsStack` —
@@ -727,8 +736,10 @@ export function lintConfig(config: any, opts: LintConfigOptions = {}): LintIssue
     // walks. ⛔ Not `stack` — that would judge un-lowered package bodies here
     // and lowered ones there, which is #16095 one layer in.
     parsed: lowered,
-    // De-duplicated against the run above, on the UNPREFIXED finding, so what
-    // reaches the list below is the set the union could not see.
+    // De-duplicated against the run above, on the UNPREFIXED finding — so what
+    // reaches the list below is what that run did not already carry under the
+    // same rule, `where`, message and non-top-level position (#18779; the key
+    // used to compare the top-level index too, and let the echo through).
     unionFindings,
     sduiManifest: opts.sduiManifest,
     loweredHookRefs,

packages/cli/src/commands/validate.ts

@@ -391,13 +391,23 @@ export default class Validate extends Command {
       //
       //     `os build` has run it since #16611; `os validate` ran the union
       //     fold and stopped, importing neither `artifactPackages` nor
-      //     `packageBodyAsStack`. `compile.ts` step 3b-ii says what survives
-      //     the de-duplication is "exactly the set the union could not see" ⇒
-      //     that whole set was findings `os build` reported and this command
-      //     structurally could not. Same FALSE-CLEAN direction #17069 fixed one
-      //     layer up, and the worse door for it: the fast inner-loop check is
-      //     what an author runs BEFORE shipping, so its clean bill of health is
-      //     the strongest false assurance the three commands can give.
+      //     `packageBodyAsStack`. Every finding this pass yields is therefore
+      //     one `os build` reported and this command structurally could not.
+      //     Same FALSE-CLEAN direction #17069 fixed one layer up, and the worse
+      //     door for it: the fast inner-loop check is what an author runs
+      //     BEFORE shipping, so its clean bill of health is the strongest false
+      //     assurance the three commands can give.
+      //
+      //     ⚠️ [#18779] This step used to size that gap by quoting `compile.ts`
+      //     step 3b-ii — "exactly the set the union could not see" — and that
+      //     sentence was FALSE when it was copied here: the de-duplication key
+      //     carried the POSITIONAL `path`, so a package-local finding and its
+      //     flattened twin got two keys and the ECHO survived. Part of every
+      //     survivor set was therefore something THIS door's own union run
+      //     already reported. The key was corrected in
+      //     `utils/artifact-packages.ts`; the gap this step closed is real and
+      //     its direction is unchanged, but ⛔ do not re-derive its size from
+      //     that sentence — it was quoted, never measured.
       //
       //     ⛔ Not a second copy of the loop — `runPerPackageAuthoringRules` is
       //     the one the build door calls, so the de-duplication key, the

packages/cli/src/utils/artifact-packages.ts

@@ -46,15 +46,62 @@ import {
   type AuthoringFinding,
 } from '@objectstack/lint';
 
+/**
+ * The leading `collection[N]` of a finding path — the ONE coordinate that
+ * differs between the two views of a single finding. `objects[3].fields.x`
+ * matches `objects[3]`; `manifest.namespace` matches nothing.
+ */
+const TOP_LEVEL_COLLECTION_INDEX = /^([A-Za-z_][A-Za-z0-9_]*)\[\d+\]/;
+
 /**
  * Identity of one finding, for the per-package de-duplication below.
  *
- * Moved here from `compile.ts` unchanged (#18677): the two doors must
- * de-duplicate identically, or "the set the union could not see" means two
- * different things depending on which command the author happened to run.
+ * Moved here from `compile.ts` unchanged (#18677); its POSITIONAL half was
+ * corrected here (#18779). All three doors must de-duplicate identically, or
+ * what survives the filter means a different thing depending on which command
+ * the author happened to run.
+ *
+ * ## Why the top-level collection index is neutralised (#18779)
+ *
+ * `rule`, `where` and `message` say WHICH finding this is; `path` says where
+ * it sits. The inherited key used `path` raw — and `path` is positional, so
+ * one finding judged twice got two keys and the `Set` below never matched
+ * them. A package body re-bases every collection from 0, while the flattened
+ * union numbers that same entry wherever `authoringRuleUnionStack` placed it:
+ * `objects[0].fields.industry` (package-local) and `objects[1].fields.industry`
+ * (union) are ONE finding under two spellings. Measured on
+ * `examples/app-multi-package` before this landed — 1 survivor, 1 echo, 0
+ * genuinely new, and `os build` printed "4 author-time warning(s)" for 3
+ * distinct ones. The de-duplication exists precisely so that "the author
+ * cannot tell a real per-package finding from an echo" would stop being true,
+ * and the positional key is why it stayed true.
+ *
+ * ⛔ The rewrite touches the KEY only — a finding's own `path` is never
+ * modified, so every door still prints the positional location it always
+ * printed. And only the TOP-LEVEL index: nested positions (`.indexes[1]`,
+ * `.columns[0]`) address the author's own document and read identically in
+ * both views, so they stay in the key and keep discriminating.
+ *
+ * ⛔ Not `nameKeyFindingPath` (`@objectstack/lint`'s runtime-gate rewrite of
+ * this same coordinate), for the reason that function's own docblock records:
+ * it is "Applied AFTER the differential, not before it … two stored items that
+ * (illegitimately) share a name must not have their distinct findings merged
+ * or cancelled by the rewrite". A de-duplication key IS that differential, so
+ * name-keying is the one place it rules itself out. Two further readings from
+ * the same docblock: its key set is DERIVED and holds `objects`, `permissions`
+ * and `books` today, so it would leave every other collection's echo standing,
+ * and it is "Exported for the pin, not for callers" — it sits on neither of
+ * that package's entries.
+ *
+ * ⚠️ What this does NOT buy, written down so the next reader does not
+ * re-inflate it: the key becomes position-insensitive, ⛔ not collision-proof.
+ * Two entries that render the same `where` — an illegitimate duplicate name —
+ * still share a key, exactly as they already did whenever their indices
+ * matched too. The claim the pass below is entitled to make is stated there,
+ * and it is narrower than "exactly the set the union could not see".
  */
 const findingKey = (f: { rule: string; where: string; path: string; message: string }): string =>
-  [f.rule, f.where, f.path, f.message].join('\u0000');
+  [f.rule, f.where, f.path.replace(TOP_LEVEL_COLLECTION_INDEX, '$1[]'), f.message].join('\u0000');
 
 /**
  * The artifact's package entries, as `{ index, id, body }` (ADR-0130 D4).
@@ -161,30 +208,55 @@ export function packageBodyAsStack(
  * ## What the asymmetry was, measured
  *
  * `os build` ran this pass; `os validate` ran the union fold and stopped,
- * importing neither seam above. `compile.ts`' own comment says what survives
- * the de-duplication is "exactly the set the union could not see" ⇒ that whole
- * set was findings `os build` reported and `os validate` structurally could
- * not. The direction is FALSE-CLEAN, and on the command an author runs BEFORE
- * shipping — the same direction and the same door #17069 fixed one layer up,
- * which is why `authoringRuleUnionStack` being in both commands did not settle
- * it. `packages/cli/test/build-json-advisory-parity.e2e.test.ts` already
- * asserted "nothing rides in build's `warnings` that validate does not also
- * report"; it stayed green because its fixture declares no `packages[]` at all,
- * so the pass it would have caught never ran there.
- *
- * ## The de-duplication key is the caller's, and it is not perfect
- *
- * `findingKey` below is `compile.ts`' key, moved unchanged: `rule`, `where`,
- * `path`, `message`. ⚠️ `path` is POSITIONAL, and a collection index in one
- * package's own body is not the index the flattened top level gives the same
- * item — so a finding on any package whose local index differs from its
- * flattened one survives the filter as an ECHO of a union finding rather than
- * as something the union could not see. Measured on `examples/app-multi-package`
- * (2 packages, `crm_account.industry`): 1 survivor, 0 of them new. ⛔ Not fixed
- * here — changing the key changes what `os build` reports, which is a separate
- * decision from making the two doors agree, and agreeing IMPERFECTLY at one
- * seam is strictly better than disagreeing at two. When it is fixed it is
- * fixed once, for both commands, which is the property this module buys.
+ * importing neither seam above. Every finding this pass yields was therefore
+ * one `os build` reported and `os validate` structurally could not — the
+ * direction is FALSE-CLEAN, and on the command an author runs BEFORE shipping.
+ * Same direction and same door #17069 fixed one layer up, which is why
+ * `authoringRuleUnionStack` being in both commands did not settle it.
+ * `packages/cli/test/build-json-advisory-parity.e2e.test.ts` already asserted
+ * "nothing rides in build's `warnings` that validate does not also report"; it
+ * stayed green because its fixture declares no `packages[]` at all, so the pass
+ * it would have caught never ran there.
+ *
+ * ⚠️ #18779 corrected the SIZE that sentence used to be given, ⛔ not its
+ * direction. #18677 and #18778 both sized this blind spot by quoting
+ * `compile.ts`' claim that the survivors are "exactly the set the union could
+ * not see" — but the key was positional, so part of every survivor set was
+ * ECHO: findings the union run ALSO reported, which means `os validate` was
+ * reporting them all along through its own union run. On
+ * `examples/app-multi-package` the whole of it was — 1 survivor, 1 echo, 0
+ * genuinely new — so `os validate`'s true blind spot on that fixture was ZERO
+ * findings, not one. ⛔ Neither card measured that; both quoted it. The
+ * asymmetry was real and worth closing on every door; its magnitude was
+ * inherited from a sentence nobody had read the definition behind.
+ *
+ * ## What the de-duplication key can and cannot promise
+ *
+ * `findingKey` above neutralises the top-level collection index (#18779), so
+ * the two views of one finding now produce one key and an echo is filtered.
+ * What reaches the lists below is therefore the set of per-package findings
+ * whose `rule`, `where`, `message` and NON-top-level position no union finding
+ * already carried.
+ *
+ * ⚠️ That is the whole claim, and it is deliberately narrower than "exactly the
+ * set the union could not see" — ⛔ do not restate it as that sentence. Two
+ * entries rendering the same `where` still collapse (see `findingKey`), and a
+ * rule that reports the same `rule`/`where`/`message` for genuinely different
+ * items distinguished ONLY by their top-level index would collapse with them.
+ *
+ * ⛔ And do not size that residue by quoting the pin next door — that move is
+ * exactly what this card exists to correct. `packages/lint/src/
+ * data-model-rule-where-slot.test.ts` holds something NARROWER than "every
+ * rule names its entity in `where`": it runs the whole registry and fails any
+ * rule that puts a BARE CONFIG PATH in `where`. That forbids the one spelling
+ * which would make the collapse systematic; it does ⛔ not promise that two
+ * entries always render different `where` strings. So the residue is MEASURED
+ * instead — over every example stack in this repo that parses today
+ * (`app-multi-package`'s built artifact, `app-crm`, `app-showcase`,
+ * `app-todo`), 45 registry rules produced 103 findings and 103 distinct
+ * neutralised keys: ZERO groups held two different raw paths. ⛔ Re-measure
+ * rather than re-quote that number — a corpus reading is a count plus the tree
+ * it was taken against, and this one was taken on a43b9d0654.
  */
 export function runPerPackageAuthoringRules(run: {
   /** Which door is asking — the same string its union run passed. */