fix: base-id membership at remaining exact-id check sites (#234) - #235
Open
ranxianglei wants to merge 2 commits into
Open
fix: base-id membership at remaining exact-id check sites (#234)#235ranxianglei wants to merge 2 commits into
ranxianglei wants to merge 2 commits into
Conversation
added 2 commits
September 9, 2026 14:48
#233 normalized prune.ts and sync.ts to compare block coverage by base id so sub-id projections (base#r0) stay covered by blocks that recorded the plain base. The same exact-string membership class remained at: - recommend.ts: isSyntheticOrPruned (protected refs + compressible ranges) - decompress.ts: findBlocksOverlappingMessages, buildRestoredContentPreview, collectBlockContent (target + nested-fold sets) - compress.ts: danglingMessageRefs, collectCoverage/preExistingCoverage (livelock guard), applySingleRange nested re-scan - boundaries.ts: message-ref lookup (base fallback), activeOwnerAnchor (owned/inherited sets), visibleBlockAnchor/blockVisibleInRange/earliestIndexOfIds (base index map, optional param - API-compatible) - report.ts: collectVisible covered set 19 regression tests (tests/subid-membership.test.ts), both projection directions plus prefix-safety controls (h_abcde not covered by h_abc). 17 fail on the pre-fix code; the 2 passing controls guard against over-matching. Stacked on PR #233 (contains its commit).
📦 Built Package ArtifactBranch: Option A — Install from npm PR tag (recommended)In your adapter project: npm install acp-kernel@pr-235Each push to this PR publishes a new version under the Option B — Download artifact
tar xzf acp-kernel-pr235.tgz
npm install ./packageThis comment is automatically updated on each push. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes #234. PR #233 normalized
prune.ts+sync.tsto compare block coverage by base id so sub-id projections (base#r0,base#call1) stay covered by blocks that recorded the plain base (and vice versa). This PR applies the samebaseIdOf/ base-set normalization to every remaining same-class site in the kernel.Stacked on #233 — this branch contains #233's commit (
ad78b45, cherry-pick ofed232f5) sobaseIdOfis available. Whichever PR merges first, the other auto-resolves. Merge #233 first if you prefer a clean single-PR history; otherwise this PR can merge directly.Sites fixed
Listed in #234's table:
src/recommend.tsisSyntheticOrPrunedblock.effectiveMessageIds.includes(message.id)activeBlockCoveredBases(state)set, tested viabaseIdOf(message.id); built once per pass incomputeProtectedRefs+buildCompressibleRangessrc/decompress.tsfindBlocksOverlappingMessagesmessageIds.has(id)messageBasesset from the param, tested viabaseIdOf(id)src/decompress.tsbuildRestoredContentPreviewb.effectiveMessageIds.includes(message.id)src/compress.tsdanglingMessageRefsblock.effectiveMessageIds.includes(rawId)src/compress.tscollectCoverage+directMessageIdsfilterbaseIdOf(id)— livelock guard now fires across formssrc/boundaries.tsactiveOwnerAnchorinherited.has(id)src/report.tscollectVisiblecoveredIds.has(message.id)Found during verification (same class, not in the table):
src/decompress.tscollectBlockContenttargetIds.has(m.id)+nestedCovered.has(m.id)src/boundaries.tsmessage-ref lookupindexByMessageId.get(rawId)baseIndexById.get(baseIdOf(rawId))fallback — stale refs from an older projection form still anchor to the visible messagesrc/boundaries.tsvisibleBlockAnchor/blockVisibleInRange/earliestIndexOfIdsresolveBoundaries+applySingleRange; exported fns gained an optional trailingbaseIndexById?param (backward-compatible, no API break; derived from the exact map when omitted)Report-only (not fixed here):
src/wire/compress-detect.ts:180boundaryRawCorecomparesblock.effectiveMessageIds(via ref map) against current core-message ids with exact equality — same class, but it's in the wire subpath (replay-guard, #91 lineage) with its own test suite; left out to keep this PR focused on the kernel path. Happy to fold it in or file a follow-up — your call.Tests
New
tests/subid-membership.test.ts— 19 regression tests covering every fixed site, both projection directions (base block / sub-id view and sub-id block / base view) plus prefix-safety controls (h_abcdeis NOT covered by baseh_abc— no accidental prefix matching). Verified against the pre-fix code: 17/19 fail, the 2 passing are the prefix-safety controls (correct in both states).Full suite: 651/651 pass,
tsc --noEmitclean, build OK.Minor note
package-lock.jsonon master is one version behindpackage.json(0.0.58 vs 0.0.59 — the release commit only bumps package.json).npm installrewrites it locally; I reverted that to keep this PR focused. Can be synced in a housekeeping commit if you want.