Skip to content

refactor(store): install_meta_belief → INSERT OR IGNORE for atomic idempotence (#772) - #773

Merged
github-actions[bot] merged 2 commits into
mainfrom
feat/issue-772-install-or-ignore
May 14, 2026
Merged

refactor(store): install_meta_belief → INSERT OR IGNORE for atomic idempotence (#772)#773
github-actions[bot] merged 2 commits into
mainfrom
feat/issue-772-install-or-ignore

Conversation

@robotrocketscience

Copy link
Copy Markdown
Owner

Closes #772. Follow-up to #755 (PR #771) per the disposition in #772.

What this PR does

Refactors MemoryStore.install_meta_belief from a SELECT-then-INSERT
idempotency check to the project's established INSERT OR IGNORE +
rowcount idiom (10 existing call sites in store.py). Eliminates the
TOCTOU window between SELECT and INSERT.

Surface

  • src/aelfrice/store.pyinstall_meta_belief body. Validation
    (ValueError raises for invalid static_default / half_life_seconds
    / unknown signal classes) runs unchanged before the INSERT. Net
    reduction: 11 lines → 8 lines, one fewer SQLite round-trip.

  • tests/test_meta_beliefs.py — new test
    test_install_concurrent_race_returns_false_not_raises exercises the
    rowcount == 0 return path: second install on the same key returns
    False, does not raise sqlite3.IntegrityError. Single-threaded
    (deterministic); the existing
    test_install_meta_belief_is_idempotent_returns_false_on_reinstall
    covers the no-race idempotency case.

Behaviour deltas

Case Before After
First install True True
Re-install (same key, sequential) False False
Concurrent install race (would-have-raced) IntegrityError raised on loser False returned on loser
Invalid config (out-of-range / unknown class) ValueError ValueError (unchanged)

Acceptance mapping (#772)

Acceptance item Where covered
install_meta_belief switches to INSERT OR IGNORE + rowcount check store.py body (commit 55e5764)
Existing test_install_meta_belief_is_idempotent_returns_false_on_reinstall still passes pytest tests/test_meta_beliefs.py → 20 passed
Existing test_install_rejects_invalid_config still passes (validation order unchanged) Same
New test exercises the would-have-raced path test_install_concurrent_race_returns_false_not_raises (commit b9f7c0d)

Verification

  • uv run pytest tests/test_meta_beliefs.py — 20 passed.
  • git log --format='%h %G? %s' github/main..HEAD — both commits signed (G).
  • git merge-base --is-ancestor github/main HEAD — FF on github/main.
  • Discretion grep on the diff — clean.

Out of scope

@robotrocketscience robotrocketscience added the author-pascal Authored by parallel session pascal label May 14, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@robotrocketscience has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 22 minutes and 21 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a606aac0-22f4-4b6d-8ca1-57ea857d46f3

📥 Commits

Reviewing files that changed from the base of the PR and between 0f0d138 and b9f7c0d.

📒 Files selected for processing (2)
  • src/aelfrice/store.py
  • tests/test_meta_beliefs.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-772-install-or-ignore

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 and usage tips.

@robotrocketscience robotrocketscience added the attn:review Needs review (PR open, awaiting reviewer) label May 14, 2026
@robotrocketscience

Copy link
Copy Markdown
Owner Author

[claim:review:curie:2026-05-14T01:41:31Z]

@robotrocketscience

Copy link
Copy Markdown
Owner Author

Review (LGTM, adding ready-to-merge).

Diff matches issue #772 exactly: SELECT-then-check → INSERT OR IGNORE + cur.rowcount > 0. Validation order preserved (ValueError raises run before the insert). Net -3 lines, one fewer SQLite round-trip.

Idiom alignment: 10 existing INSERT OR IGNORE call sites in store.py, multiple rowcount > 0 / or 0 returns — this fits.

Test: test_install_concurrent_race_returns_false_not_raises exercises the rowcount==0 path single-threaded as the acceptance item specifies. The try/except sqlite3.IntegrityError → AssertionError framing makes the regression-mode obvious if the idiom drifts back.

Verification:

  • FF on github/main
  • Both commits SSH-signed (G) ✓
  • pytest (3.12) + pytest (3.13) SUCCESS ✓
  • Staging gate (secrets / pattern / history / commit-msg / pr-title / pr-body) SUCCESS ✓
  • CodeQL (python + actions) SUCCESS ✓
  • deptry / vulture / typos SUCCESS ✓
  • Discretion grep on diff vs main: clean ✓

Minor observation, non-blocking: encode_signal_weights(signal_weights) now runs on the no-op re-install path (it short-circuited before). It's pure / cheap so no functional concern.

@robotrocketscience robotrocketscience added ready-to-merge Trigger merge-train: FF main to this PR's head and removed attn:review Needs review (PR open, awaiting reviewer) labels May 14, 2026
@robotrocketscience

Copy link
Copy Markdown
Owner Author

[release:review:curie:2026-05-14T01:43:33Z]

@github-actions
github-actions Bot merged commit b9f7c0d into main May 14, 2026
31 of 34 checks passed
@github-actions github-actions Bot removed the ready-to-merge Trigger merge-train: FF main to this PR's head label May 14, 2026
@github-actions

Copy link
Copy Markdown

merge-train: merged b9f7c0dmain via FF push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-pascal Authored by parallel session pascal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(store): install_meta_belief → INSERT OR IGNORE for atomic idempotence (post-#755)

1 participant