Skip to content

test(l1): pin verified-msgpack refusals and fix stale comments (LAB-11867) - #665

Merged
27Bslash6 merged 1 commit into
mainfrom
agent/miss-huang/lab-11867-l1-test-gaps
Oct 30, 2026
Merged

27Bslash6 merged 1 commit into
mainfrom
agent/miss-huang/lab-11867-l1-test-gaps

Conversation

@27Bslash6

Copy link
Copy Markdown
Contributor

Test-and-comment follow-up to the L1 verified-MessagePack change. No executable src/ line changes.

  • decorators/wrapper.py: the comment on the L1 hit site's plaintext branch said every swapped-in entry fails as a corrupt frame. An encrypted handler refuses it as SuspiciousCacheEntryError; the comment now says so.
  • cache_handler.py: drop two # noqa: SLF001 markers. SLF is not in the ruff select list, so they suppressed nothing. The # type: ignore[union-attr] markers stay.
  • tests/unit/test_l1_verified_msgpack.py:
    • each row of the refusal test pins its own exception and message with pytest.raises(..., match=...), the encrypted SuspiciousCacheEntryError row included;
    • a new test asserts set(_frames()) == set(_FRAME_NAMES), so a frame added to _frames() alone fails instead of silently skipping the parity matrix (checked by adding a key to _frames() only: the assert fails);
    • the autouse env fixture no longer calls reset_settings(); the root conftest already brackets every test with it;
    • a comment block that broke mid-sentence is re-wrapped.

Verification: ruff check, ruff format --check, and pytest tests/unit/test_l1_verified_msgpack.py (36 passed) both with and without CACHEKIT_MASTER_KEY exported.

Closes LAB-11867

…mment

- wrapper.py: the L1 hit site's plaintext branch comment now says an
  encrypted handler refuses a swapped entry as SuspiciousCacheEntryError
- cache_handler.py: drop two noqa SLF001 markers; SLF is not selected
- tests: pin every refusal message per handler row, assert the frame
  name list matches _frames(), drop reset_settings calls the root
  conftest already makes, and re-wrap a broken comment block
@kodus-27b

kodus-27b Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Summary by CodeRabbit

  • Bug Fixes
    • After an L1 cache hit, verified MessagePack data is now retained in its verified representation rather than the original bytes.
    • Updated error descriptions for swapped cache entries to identify suspicious encrypted entries or corrupt frames.
    • Improved checks for handler errors and frame-name coverage.

Walkthrough

This change removes two lint suppressions from verified MessagePack calls, clarifies the L1 swapped-entry comment, and updates tests for handler refusal errors, frame-name coverage, and fixture setup.

Changes

Verified MessagePack L1 checks

Layer / File(s) Summary
L1 checks and test coverage
src/cachekit/cache_handler.py, src/cachekit/decorators/wrapper.py, tests/unit/test_l1_verified_msgpack.py
The handler calls no longer suppress SLF001. The L1 comment distinguishes encrypted entries from corrupt frames. Tests check expected refusal errors for both deserialisation methods, frame-name coverage, and fixture setup. Replay-test comments are reflowed.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related PRs

Suggested reviewers: kodus-27b

Merge Risk: Minimal · up to 52018

No actionable merge-blocking risk remains in this test and comment update.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title states a test and comment change for the L1 verified-MessagePack path, with a ticket reference. It matches the changeset: the test file pins refusal exceptions and adds a frame-parity assert…
Description check Passed The description is mostly complete and follows the intent of the template. It gives a clear per-file account of the changes, and it reports verification steps (ruff check, ruff format --check, and 36 …
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title states a test and comment change for the L1 verified-MessagePack path, with a ticket reference. It matches the changeset: the test file pins refusal exceptions and adds a frame-parity assertion, and the comments are corrected. The title is 74 characters, within a reasonable length, and it names the main change. Minor point: 'fix stale comments' covers a second, smaller part of the change, which is expected.

Full details: Description check

Explanation

The description is mostly complete and follows the intent of the template. It gives a clear per-file account of the changes, and it reports verification steps (ruff check, ruff format --check, and 36 passing tests, with and without CACHEKIT_MASTER_KEY). It links the ticket (LAB-11867). Some template sections are missing: there is no explicit 'Description' heading, no 'Type of Change' selection (it is a test and comment change, which would map to 'Refactoring' or none), no Security Checklist confirmation, and no Documentation or Backward Compatibility status. The Motivation is implied by the test-hardening intent but not stated as a separate section. These gaps are non-critical for a test-only change, so the check passes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 3 files.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown
Contributor

Instruction budget: PASS

Head 5201879b6a7f4e40f4ca7ee8edd1daeb596a1452 measured against base 4db94e89a1edb1794fe457210d0e07a073091502 on the same runner, on cpython-3.12-x86_64 (3.12.3). Workflow run

Verdict Path Head Ir/op Base Ir/op Delta
ok byte_storage_store_40b 2,756 2,755 +0.04%
ok byte_storage_store_64k 724,721 724,720 +0.00%
ok file_set 86,793 86,794 -0.00%
ok l1_hit 13,130 13,129 +0.01%
ok l2_hit 196,188 196,197 -0.00%
ok l2_hit_async_metrics 195,962 195,963 -0.00%
ok minimal_l1_hit 11,985 11,986 -0.01%
ok miss 242,012 242,013 -0.00%
ok object_cache_put_full 198,445 198,446 -0.00%
ok secure_l1_hit 156,380 156,384 -0.00%
ok serializer_arrow 2,220,922 2,220,309 +0.03%
ok serializer_auto 72,749 72,749 +0.00%
ok serializer_default 64,518 64,520 -0.00%
ok serializer_default_records 1,481,791 1,481,785 +0.00%
ok serializer_encrypted 109,607 109,605 +0.00%
ok serializer_orjson 24,857 24,854 +0.01%

A path fails at +1% or more against the base (+2% for serializer_orjson). This check is not required: a deliberate cost increase is accepted at merge. Method: docs/performance.md, Instruction budgets.

@27Bslash6
27Bslash6 merged commit ebe3b5c into main Oct 30, 2026
51 checks passed
@27Bslash6
27Bslash6 deleted the agent/miss-huang/lab-11867-l1-test-gaps branch October 30, 2026 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant