test: add lazy mutation property coverage#105
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 45 minutes and 54 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive stateful property test suite for qjson lazy table proxy mutation and encoding semantics, alongside build and CI integration. The test suite uses an independent ordered oracle model, randomized state-machine operations, checkpoint assertions, and deterministic regressions to validate behavior across mutation, materialization, and encoding paths. ChangesLazy Mutation Property Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/lua-lazy-mutation-property.yml (1)
44-46: ⚡ Quick winInclude
Cargo.lockin the cache keyThe workflow caches
~/.cargo/*andtargetusing onlyhashFiles('Cargo.toml', 'Makefile'), so lockfile-only dependency changes won’t bust stale Rust build cache state. HashCargo.locktoo.Suggested fix
- key: mut-prop-${{ runner.os }}-${{ hashFiles('Cargo.toml', 'Makefile') }} + key: mut-prop-${{ runner.os }}-${{ hashFiles('Cargo.toml', 'Cargo.lock', 'Makefile') }}🤖 Prompt for 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. In @.github/workflows/lua-lazy-mutation-property.yml around lines 44 - 46, The cache key currently hashes only Cargo.toml and Makefile, so add Cargo.lock to the hashFiles call used in the cache key expression (the line with key: mut-prop-${{ runner.os }}-${{ hashFiles('Cargo.toml', 'Makefile') }}) so that lockfile-only dependency changes invalidate the cache; update that same key expression to include 'Cargo.lock' in the hashFiles argument while leaving the restore-keys pattern (mut-prop-${{ runner.os }}-) as-is.
🤖 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 @.github/workflows/lua-lazy-mutation-property.yml:
- Around line 28-30: Update the workflow so action refs are pinned to immutable
commit SHAs instead of tags for actions/checkout@v4, actions/cache@v4 and
leafo/gh-actions-lua@v13; add persist-credentials: false to the actions/checkout
step (since the workflow does not push) and modify the cache key used by
actions/cache to compute hashFiles(...) including "Cargo.lock" (in addition to
existing files) so the cargo cache invalidates when dependencies change; ensure
you replace the three tagged refs with their corresponding full commit SHAs in
the YAML.
In `@docs/superpowers/specs/2026-05-31-lazy-mutation-property-tests-design.md`:
- Around line 37-40: The spec overstates sparse-array support: the test model
tests/lua/lazy_mutation_property_spec.lua serializes arrays with ipairs so nil
holes cannot be represented in the oracle; either update the test model to use
an explicit hole sentinel when materializing/serializing arrays (replace use of
ipairs with a serializer/deserializer that preserves a sentinel for holes and
update any oracle equality checks), or change the spec text to remove the claim
of full sparse-array support and instead state that sparse arrays are only
handled deterministically via regression (no true nil holes preserved).
Reference the test file lazy_mutation_property_spec.lua and the use of ipairs
when applying the fix.
In `@tests/lua/lazy_mutation_property_spec.lua`:
- Around line 684-703: The test currently reports only the prefix length from
shortest_failing_prefix but not the minimized failure itself; after computing
prefix = shortest_failing_prefix(SEED, case_no, STEPS) call execute_case(SEED,
case_no, prefix) (wrapped in pcall) to re-run the minimized prefix and capture
its error string, then include that minimized error text in the final error
message alongside the "shortest_failing_prefix=..." text so the CI log contains
the actionable minimized failing trace; update the block that computes prefix
and builds shrink to perform this re-run and append the caught error (from pcall
on execute_case) to the error() call.
- Around line 355-393: The assert helpers currently call fail_message with only
a generic label; update assert_model_value and assert_checkpoint to include
serialized representations of both expected and actual values in the
fail_message so CI shows the diverging payloads: when comparing in
assert_model_value (the semantic_equal check) append the expected JSON (use
model_encode or a safe serializer) and the actual value (materialized via
qjson.materialize when needed) to the message; in assert_checkpoint, for each
assertion (materialized vs model, encoded vs expected_json, cjson_value vs
model, reparsed vs model) include the expected model JSON and the actual
serialized/encoded result (use qjson.encode, model_encode, and a string form of
cjson_value/reparsed after materializing) in the fail_message so failures
display both expected and actual payloads. Ensure you reference and reuse
qjson.materialize, qjson.encode, cjson_decode_with_array_mt, qjson.decode,
model_encode, semantic_equal, and fail_message when building the messages.
---
Nitpick comments:
In @.github/workflows/lua-lazy-mutation-property.yml:
- Around line 44-46: The cache key currently hashes only Cargo.toml and
Makefile, so add Cargo.lock to the hashFiles call used in the cache key
expression (the line with key: mut-prop-${{ runner.os }}-${{
hashFiles('Cargo.toml', 'Makefile') }}) so that lockfile-only dependency changes
invalidate the cache; update that same key expression to include 'Cargo.lock' in
the hashFiles argument while leaving the restore-keys pattern (mut-prop-${{
runner.os }}-) as-is.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16abe86c-08ba-4345-9d7c-b104d70ef3a6
📒 Files selected for processing (5)
.github/workflows/lua-lazy-mutation-property.ymlCONTRIBUTING.mdMakefiledocs/superpowers/specs/2026-05-31-lazy-mutation-property-tests-design.mdtests/lua/lazy_mutation_property_spec.lua
This was referenced May 31, 2026
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
1 participant
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.
Summary
Closes #104
Test Plan
Summary by CodeRabbit
Documentation
Tests
Chores