Repository navigation
Catch writes from other connections and watermark syncs before trusting decimal provenance - #46
Merged
Merged
Conversation
… missed A third external review found that provenance could still be lost. All four findings reproduced (7 of the 8 new tests fail on 188213c; the eighth is a positive control). - Writes from another SQLite connection are invisible to total_changes. The audit now also compares PRAGMA data_version, so a 0.2 committed by another connection beside an admitted 0.1 is refused, not summed to 0.3. - set_watermark() and create_and_seed_table() audit before they sync the trusted counters, so a public adapter call can no longer absorb an unobserved write. apply_write_event syncs its own write before writing its watermark through an internal, non-auditing path. - Cold MIN/MAX no longer converts floats through Decimal(str()); like the row path it refuses them as float_not_exact (bool as invalid_evidence_type), and process_or_rebuild returns the UNSUPPORTED uncacheable outcome for extremum and top-k rebuild refusals too. - An unregistered float GROUP BY value raises FloatNotExact, so it reaches the typed outcome instead of escaping as a bare TypeError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194kAmYv4pLMyrspzKEWExZ
The third external review of the float PR (#45) found that two provenance bypasses survived its merge. Both reproduce on main; the tests fail there. - Writes from another SQLite connection are invisible to total_changes. The audit now also compares PRAGMA data_version, so a 0.2 committed by another connection beside an admitted 0.1 is refused, not summed to 0.3. - set_watermark() and create_and_seed_table() audit before they sync the trusted counters, so a public adapter call can no longer absorb an unobserved write. apply_write_event syncs its own write first, then writes its watermark through an internal, non-auditing path. - SortedMultisetExtremumCache carries uncacheable_reason, set when process_or_rebuild returns the UNSUPPORTED outcome for an extremum refusal and cleared by a clean rebuild, as AggregateCache does. The review's MIN/MAX and float GROUP BY findings were already fixed on main by #45's fourth commit; their tests here pass on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194kAmYv4pLMyrspzKEWExZ
…ow-up Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194kAmYv4pLMyrspzKEWExZ
Owner
Author
|
Live Database Containers is red, but the failure is not from this PR.
Generated by Claude Code |
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.
🤖 Generated with Claude Code
This is a follow-up to #45, from the third external review of that PR (at 188213c). Two of its four findings survived the merge. Each reproduces on
main, and the new tests fail there.total_changes, so a 0.2 committed elsewhere beside an admitted 0.1 summed toDecimal('0.3')PRAGMA data_version, which moves on any other connection's commit. Exact-decimal eligibility is revoked for good, because the lost source types cannot be recovered, only refusedset_watermark()synced the trusted counter without auditing first, so it absorbed an unobserved write.create_and_seed_table()did the sameapply_write_eventsyncs its own write, then writes its watermark through an internal path that does not auditDecimal(str())SortedMultisetExtremumCache.uncacheable_reasonis now set on the UNSUPPORTED outcome, as forAggregateCacheTypeErrormainOwnership contract. The adapter owns the substrate's writes.
Evidence (local, Python 3.13)
.venv/bin/python -m pytest -q -p no:cacheprovider --ignore=tests/production_native.pyexits 0.mypy smplcachereports no issues, andblackis clean.tests/test_source_provenance.py.main, 4 fail: the other connection, the watermark, the seed and the extremum mark.Branch note. This branch reused #45's name after the merge. Its diff against
mainis only these 3 files, and the extra commits in the list are #45's pre-squash history.https://claude.ai/code/session_0194kAmYv4pLMyrspzKEWExZ
Generated by Claude Code