You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 031e5fb
Browse filesBrowse the repository at this point in the historyBrowse files
fix(cli): the per-package de-duplication key ignores the top-level collection index, so an echo no longer survives it (#18878)
Fixes#18779
## The card's central reading, reproduced first
Measured on `origin/main` `a43b9d0654` over the repo's own two-package
fixture
`examples/app-multi-package`, `os build --json` exiting 0:
```
warnings: 4
[0] field-no-consumers object "crm_order" · field "account" objects[0].fields.account
[1] field-no-consumers object "crm_order" · field "amount" objects[0].fields.amount
[2] field-no-consumers object "crm_account" · field "industry" objects[1].fields.industry
[3] field-no-consumers package 'com.example.multi.core' —
object "crm_account" · field "industry" objects[0].fields.industry
```
`[3]` is `[2]`, re-reported at the package-local index. Same rule, same
entity,
same message; the only difference is the top-level collection index,
which is
the one coordinate `findingKey` had no business comparing. **1 survivor,
1 echo,
0 genuinely new** — the card's reading holds, and the pass's strongest
available
value statement was carried entirely by a duplicate.
## Which option I took, and what the rejected one would have cost
I fixed **the key**, and the comments with it.
Fixing only the comments was the cheaper option and it was rejected on a
measurement, not a preference: the de-duplication exists so that "the
author
cannot tell a real per-package finding from an echo" would stop being
true, and
on the one fixture anyone can check, it was still true. Correcting the
prose
would have left `os build` printing `4 author-time warning(s)` for 3
distinct
ones, and left the repo with an accurate comment describing a filter
that does
not filter. The cost of the option I took is that `os build` / `os
validate` /
`os lint` report one fewer line on such a project, which is an
observable change
and is why this carries a changeset.
Triage settled the blocker: the "`os build` output stays byte-identical"
constraint was #18769's **PR contract, not a product contract**, and its
scope
ended with that PR.
## Measured: exactly what stops being reported
Same fixture, all three doors, the key reverted and restored in place
(on-disk
blob hash asserted both ways, tree verified clean 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 line that stops being reported is the echo above. **No input's
verdict
moves**, and that is structural rather than a property of this fixture:
every
finding the de-duplication drops has, by construction, a finding with
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. `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.
## Clause-②
Clause-②: no
Derived from the measured diff, not inherited. The dispatching claim
left this
blank deliberately and expected `yes (widening)` on the reasoning "fewer
findings reported ⇒ `--strict` refuses less". Measured, `--strict` does
**not**
refuse less: `failing` drops 4 to 3 and the verdict stays `exit 1`,
because the
dropped line's twin is still counted. No accept set moves in either
direction,
and the diff adds no schema key, closed-set member, published export or
registry
entry. Declaration carrier: `Clause-②-correction: 5723810598` on the
card.
## The falsified sentence — all carriers, re-derived
`git grep "set the union could not see"` on `a43b9d0654` finds more than
the
four the dispatch named. Source **and** tests, all corrected here:
| file | treatment |
|---|---|
| `packages/cli/src/commands/compile.ts` | claim corrected, quote kept |
| `packages/cli/src/commands/lint.ts` | claim corrected, quote kept |
| `packages/cli/src/commands/validate.ts` | claim corrected, quote kept
|
| `packages/cli/src/utils/artifact-packages.ts` | claim corrected at the
definition |
| `packages/cli/test/lint-per-package-authoring-parity.test.ts` | claim
corrected |
| `packages/cli/test/lint-per-package-authoring-seam.test.ts` | claim
corrected |
| `packages/cli/test/validate-per-package-authoring-parity.test.ts` |
claim corrected |
| `packages/cli/test/validate-per-package-authoring-seam.test.ts` |
claim corrected |
Each keeps the sentence as a **quotation being corrected** rather than
deleting
it, so the next reader meets the correction where they would have met
the claim.
## TWO pins were being held up by the echo
Both are the same shape and both were repaired the same way — by giving
the
fixture a survivor the union genuinely cannot see, never by relaxing an
assertion.
**1. `validate-per-package-authoring-parity.test.ts`** (#18677). Its
non-vacuity case went red: the planted fixture's only per-package
survivor was
the echo, so filtering it left the parity cases comparing two empty
sets.
**2. `build-text-face-advisory-count.test.ts`** (#18780) — found by CI,
not
locally, and the local gap was mine: `packages/cli`'s `test` script is a
bare
`vitest run` with **no `--project` filter**, so CI runs both projects,
while I
had run `--project unit` plus only the two integration files this diff
touches.
This file is in the integration project and was never collected. Its lit
control asserts the fixture reaches the per-package pass and leaves a
survivor:
```
AssertionError: expected 0 to be greater than 0
test/build-text-face-advisory-count.test.ts:239
> the fixture reaches the per-package pass and raises a survivor there
```
`bc_account.industry` had no consumer anywhere, so the union raised it
too and
the "survivor" was that finding re-reported at the package-local index.
`orders`
now owns the view that displays it. ⛔ `toBeGreaterThan(0)` is untouched
— it is
what stops #18780's equality pins from going vacuous.
Both preconditions now additionally assert the survivor's **pedigree**
(the
field is named only behind the per-package prefix, and no union finding
names
it), so neither control can be silently re-lit by a duplicate. The
`warnings: 4`
reading in #18780's header is kept as the record of the defect it
measured, with
a note that the same fixture reports 3 since this change.
## What the key does not buy, measured rather than quoted
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.
That residue is measured, not sized by citing a neighbouring pin — which
is the
move 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 fails any rule that
puts a
**bare config path** in `where`. 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, 103 findings, **103
distinct
neutralised keys, 0 collisions**, on `a43b9d0654`.
## Verification
- **Pin red before / green after.**
`test/per-package-dedup-positional-echo.test.ts`,
key reverted to base in place: `2 failed | 4 passed` (the ECHO and
REAL_UNION
cases). Key restored: `6 passed`. On-disk mutation proved by blob hash
both
ways; the test imports the mutated module through a relative source
specifier,
so no build sits between mutation and assertion.
- **The control can fail.** An ablation widening the rewrite from the
top-level
index to *every* index turns exactly one case red — the NESTED control.
The
first draft of that control was built on a `field-no-consumers` twin and
stayed **green** under the same ablation, so the fixture now carries a
bare
`unique: true` index to give the control a finding whose path really has
a
nested index. A control that cannot fail is decoration.
- **Tier measured in both directions**, from the predicate rather than
the
filename: the new pin fires no integration signal and is absent from the
derived integration population — UNIT tier, asserted by the file's own
last
case.
- **Changeset decided by measuring the built `dist`**, with controls
both ways:
the rewritten key is present in
`packages/cli/dist/utils/artifact-packages.js`
and `files[]` ships `dist`; positive control
`runPerPackageAuthoringRules`
present; negative control (a test-only symbol) 0 hits. Published ⇒
changeset,
graded `patch`.
- `dispatch-gates.mjs`: **61 derived families, 61 run, 0 NOT-MEASURED, 0
UNRUN**
(exit codes recorded, none is 3).
- `pnpm lint` (`eslint . --no-inline-config`, whole repo, not narrowed):
**exit 0** at `6a0a4df1b4`.
- `pnpm --filter @objectstack/cli test` run **WHOLE** — no `--project`,
no
file list, exactly what CI runs: **267 files / 3485 tests passed, exit
0**.
The same command at `15cc0db8d4` reproduced CI's red first:
`3 failed | 264 passed (267)`, of which the one real assertion was
#18780's
lit control (the other two were this worktree lacking
`packages/cli/dist`,
which that pin refuses by name; cleared by building the package).
- `pnpm --filter @objectstack/cli typecheck`: exit 0.
`check:dual-build-cjs-loads` first returned exit 3 — its own text says
"This is
NOT a pass: nothing was measured", 8 packages had no `dist` in this
worktree. I
built them and re-ran it: exit 0.
## Acceptance notes
- **Two pending changesets still assert the falsified sentence** and are
**not**
touched here: `.changeset/18677-validate-per-package-authoring-pass.md`
and
`.changeset/18778-lint-per-package-authoring-pass.md`.
`check:empty-changeset`
refuses a PR that modifies a changeset present on the merge base, and
names
this exact situation as its DELIBERATE CORRECTION class, whose remedy is
"do
NOT restore it; get it confirmed on the PR". That confirmation is the
reviewing seat's to give, so the call is surfaced here rather than
taken. The
correction itself is published in this PR's own changeset, which lands
in the
same release. Note also that #18677's changeset says "After: both report
4",
which this change makes read 3.
- Derivation ran against a tree behind `origin/main` (the derivation's
own
staleness warning named `scripts/gen-sdui-manifest-node.mjs` among
others);
CI judges the merge.
- Noted, not filed: `bin/run-dev.js` needs `TSX_TSCONFIG_PATH` pinned
when
invoked from an example directory, and says so itself in a good refusal
— a
working command, not a defect.
Authored by Claude Code in session `session_01DvvamiacK328idtBYJBxV3`
(durable attribution kept in prose: the body-edit channel appends its
own footer block, so a footer sent here would be stored twice).
---
_Generated by [Claude Code](https://claude.ai/code)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
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
6
+
7
+
`runPerPackageAuthoringRules` runs the author-time rule table once per
8
+
`packages[]` entry and drops anything the union run already reported. Its key
9
+
was `rule` + `where` + `path` + `message`, and `path` is **positional**: a
10
+
package body re-bases every collection from 0, while the flattened union numbers
11
+
that same entry wherever `authoringRuleUnionStack` placed it.
12
+
`objects[0].fields.industry` and `objects[1].fields.industry` are ONE finding
13
+
under two spellings, so the `Set` never matched them and the echo survived the
14
+
filter that exists to remove it.
15
+
16
+
Measured on `origin/main` a43b9d0654 over the repo's own two-package fixture
17
+
`examples/app-multi-package`, at every door, before and after:
0 commit comments