Fix #377: Signal bitmap shard corruption

[flyingrobots]

What changed

Added explicit corruption handling for malformed bitmap shard shapes and non-byte bitmap values.

Why

In non-strict mode, corrupted shards still return an empty result, but now emit warnings so callers and tests can distinguish corruption from a real no-neighbor result. In strict mode, invalid bitmap values now throw ShardCorruptionError.

Closes #377

Validation

• npx vitest run test/unit/domain/services/BitmapIndexReader.test.ts
• npm run typecheck:src -- --pretty false
• npm run typecheck:test -- --pretty false

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling and validation when loading and processing data to enhance system reliability and data integrity checks.

• Tests

  • Expanded test coverage for error handling scenarios to ensure proper behavior in edge cases and data corruption scenarios.

[coderabbitai]

Warning

Review limit reached

@flyingrobots, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 48 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c8bea599-2555-4003-a731-8eaa1cf3220a

📥 Commits

Reviewing files that changed from the base of the PR and between aaba18b and 6017968.

📒 Files selected for processing (2)

• src/domain/services/index/BitmapIndexReader.ts
• test/unit/domain/services/BitmapIndexReader.test.ts

📝 Walkthrough

Walkthrough

BitmapIndexReader now performs explicit validation on decoded shard payloads and bitmap values. Two new error handlers consolidate invalid-data behavior: _handleInvalidShardShape and _handleInvalidBitmapValue create ShardCorruptionError with standardized reason codes and log in non-strict mode. Shard decoding and edge extraction routes invalid data to these handlers instead of silently returning empty results.

Changes

Enhanced validation and error handling for corrupted shards

Layer / File(s)	Summary
Error handlers and shard shape validation src/domain/services/index/BitmapIndexReader.ts	New _handleInvalidShardShape and _handleInvalidBitmapValue helpers consolidate error handling with standardized ShardCorruptionError reason codes. _decodeAndCacheShard now validates decoded payloads are non-null plain objects (not arrays) and delegates invalid shapes to the handler before caching.
Bitmap value validation in edge extraction src/domain/services/index/BitmapIndexReader.ts	_getEdges explicitly skips undefined/null bitmap entries, validates each bitmap value is a Uint8Array (delegating invalid types to the handler), and skips empty bitmaps before deserializing edge IDs.
Test assertions for error behavior test/unit/domain/services/BitmapIndexReader.test.ts	Tests inject mock loggers and verify warn is called with operation context (loadShard, deserializeBitmap) and exact reason codes (shard_not_object, bitmap_value_not_bytes). Strict-mode tests now expect ShardCorruptionError to be thrown instead of silently returning empty results.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A shard once lost in silent night,
Now speaks its truth with reason code!
No more the empty ghost array—
When data breaks, we log the way,
With handlers clear, our path shines bright. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description covers what changed, why it matters, and validation steps. However, it does not follow the provided template structure with required sections like 'Issue', 'Test plan', and 'ADR checks'.	Restructure the description to match the template: add an explicit 'Issue' section referencing #377, expand 'Test plan' as a dedicated section, and include the 'ADR checks' checklist with appropriate selections.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix #377: Signal bitmap shard corruption' directly references the linked issue and clearly summarizes the main change: adding explicit corruption signaling.
Linked Issues check	✅ Passed	The PR implements the core requirement from #377: corrupted shards now emit warnings in non-strict mode and throw errors in strict mode, distinguishing corruption from genuine no-neighbor results.
Out of Scope Changes check	✅ Passed	Changes are limited to BitmapIndexReader corruption handling and related tests. No unrelated modifications to other systems or unrelated refactoring detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-377-bitmap-reader-corruption-signals

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Release Preflight

• package version: 18.0.0
• prerelease: false
• npm dist-tag on release: latest
• npm pack dry-run: passed
• jsr publish dry-run: passed

If you tag this commit as v18.0.0, release workflow will publish.

[flyingrobots]

@codex Self-discovered Code Lawyer issue during PR audit.

Severity	Source	File	Lines	Issue	Recommended mitigation
P2 Major	Self	test/unit/domain/services/BitmapIndexReader.test.ts	changed constructor calls in corrupt shard recovery tests	The PR adds new as any constructor casts for BitmapIndexReader options. Repo policy rejects new any anywhere; these casts make the regression test depend on type escape instead of the real constructor contract.	Replace the added as any call sites with the actual BitmapIndexReader constructor option shape or a typed helper/factory, then rerun the focused BitmapIndexReader test, npm run typecheck:test, ESLint, and diff check.

[flyingrobots]

@codex Activity Summary

Issue	Severity	Source	File	Commit	Outcome
Removed newly introduced as any constructor casts and completed the logger test double with LoggerPort.child.	P2 Major	Self	test/unit/domain/services/BitmapIndexReader.test.ts	aaba18b5	Fixed and pushed.

Validation:

Check	Result
npm exec vitest run test/unit/domain/services/BitmapIndexReader.test.ts	Pass: 29 tests
npm run typecheck:test	Pass
npm run lint -- test/unit/domain/services/BitmapIndexReader.test.ts	Pass
git diff --check	Pass
Pre-push IRONCLAD gates	Pass: 535 test files, 7199 tests

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/domain/services/index/BitmapIndexReader.ts`:
- Around line 256-258: The current guard in BitmapIndexReader (checking data ===
null || typeof data !== 'object' || Array.isArray(data')) still accepts
non-plain objects (e.g. Uint8Array) which can be cached as a LoadedShard and
later misinterpreted by _getEdges; update the validation to only accept plain
objects (e.g. ensure Object.getPrototypeOf(data) === Object.prototype or use a
shared isPlainObject helper) and call this._handleInvalidShardShape(path, oid,
'shard_not_object') when the prototype check fails so non-plain decoded values
are rejected before caching.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 88b0c976-abe5-4a89-bca7-97d194b656c4

📥 Commits

Reviewing files that changed from the base of the PR and between a21fb8b and aaba18b.

📒 Files selected for processing (2)

• src/domain/services/index/BitmapIndexReader.ts
• test/unit/domain/services/BitmapIndexReader.test.ts

[github-actions]

Release Preflight

• package version: 18.0.0
• prerelease: false
• npm dist-tag on release: latest
• npm pack dry-run: passed
• jsr publish dry-run: passed

If you tag this commit as v18.0.0, release workflow will publish.

[flyingrobots]

@codex Code Lawyer Activity Summary

Issue	Severity	Source	File	Commit	Outcome
Top-level CBOR byte-string shards could pass the decoded-shard shape guard, be cached as LoadedShard, and later look like an empty shard without warning.	P1 High	PR review thread / CodeRabbit	src/domain/services/index/BitmapIndexReader.ts	60179683	Fixed by accepting only plain decoded shard objects before cache insertion. Review thread resolved.

Validation:

Check	Result
RED: npm exec vitest run test/unit/domain/services/BitmapIndexReader.test.ts -t "rejects top-level byte-string shards before caching them" before fix	Failed as expected: corrupt byte-string shard was cached and read once
GREEN: npm exec vitest run test/unit/domain/services/BitmapIndexReader.test.ts	Pass: 30 tests
npm run typecheck:test	Pass
npm run lint -- src/domain/services/index/BitmapIndexReader.ts test/unit/domain/services/BitmapIndexReader.test.ts	Pass
git diff --check	Pass
Pre-push IRONCLAD gates	Pass: 535 test files, 7200 tests

[github-actions]

Release Preflight

• package version: 18.0.0
• prerelease: false
• npm dist-tag on release: latest
• npm pack dry-run: passed
• jsr publish dry-run: passed

If you tag this commit as v18.0.0, release workflow will publish.