Repository navigation
bench: BASELINE-LEDGER rows for the v0.23.2 security/privacy train - #412
Conversation
#366 #370 #374 #384/#391 #155) Append-only boundary rows for every main-tree merge since the last ledger reconciliation that shapes recall-path content, the compaction path, or the eval instrument. Merge shas derived with gh pr view --json mergeCommit; file:line facts checked against origin/main c98cb6a. The single discharge event for the OPEN rows is the F53 re-bank registration (#367/#380) on the shipped posture (durable lossless + provider-copy transform), which registers as a new row. Refs #377 #367 #380 #252.
|
Warning Review limit reachedNext included review available in 8 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #412 - bench: BASELINE-LEDGER rows for the v0.23.2 security/privacy train
Head: 4e70c417e0eb2a9a55d77afa6aaaf222f252c685 into main. Review event: COMMENT.
Estimated review effort: 1/5 (~12 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
bench/BASELINE-LEDGER.md |
modified | +14/-0 | Documentation | Low |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Maintainer Analysis
Changed behavior:
- Adds six BASELINE-LEDGER entries documenting expected benchmark effects and re-baseline obligations for merged changes #364, #366, #370, #374, #384/#391, and #155.
- Identifies the F53 re-bank as the planned discharge event for privacy-related OPEN rows and records expected-inert conditions for concurrency and PEM-scanner changes.
Affected invariants:
- Benchmark ledger entries should accurately bind behavioral boundaries to merge commits, affected configurations, and measurable discharge criteria.
- Security/privacy changes must fail closed without silently leaking provider-bound private-key material.
- Lossless durable storage remains the default unless sensitive-pattern redaction is explicitly enabled.
Evidence:
- bench/BASELINE-LEDGER.md:46 introduces the v0.23.2 security/privacy train ledger section.
- bench/BASELINE-LEDGER.md:53-58 contain the six added boundary rows and their stated re-baseline status.
Limitations:
- Review was limited to the supplied documentation diff; merge SHAs, issue relationships, cited source locations, configuration defaults, benchmark receipts, and F53 results could not be independently verified.
- No commands, tests, builds, project scripts, or PR code were executed.
No-finding rationale: The diff changes only benchmark-accounting documentation. Its rows provide concrete merge identities, affected surfaces, expected benchmark effects, and discharge conditions. No internally validated correctness, security, data-loss, CI, or release regression was established from the supplied diff alone.
Risk Taxonomy
No finding categories.
Validation and Proof
Documentation-only changed surface; runtime execution proof is not required by default.
- recommended: Docs/readme review - All reviewed files are documentation paths. Proof: PR description notes the docs intent.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Related Context
Related issues/PRs: #364, #366, #370, #374, #384, #391, #155, #377.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e70c417e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…; run-path transform pointer; assignment-pattern wording Audit finding on #412 (opus-fast-worker, P2): the #370 row credited that merge with #367 asks 1+3; #370 touches no ledger file — ask 1 (the privacy-trio boundary row) landed via #368 b2142ed. Also point the harness citation at evaluate_question (the benchmark run path) alongside the helper call sites, and name the ordering peer 'assignment patterns' as the code and #365 do.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a5210920a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nsform-change count, not a blocked counter Review thread on #412: a redaction changes provider-bound text without blocking, and the harness has no privacy_blocked field, so 'privacy_blocked == 0' cannot verify the #366 / #384-#391 boundaries as inert. The rows now require the transform-change count (documents where protect_embedding_text returned changed=True or raised, measured by a full prewarm pass = input-hash parity) as the discharge measurement.
There was a problem hiding this comment.
Walkthrough
PR: #412 - bench: BASELINE-LEDGER rows for the v0.23.2 security/privacy train
Head: e3d62858d9ec06cd0d4a96d42430f963639f1fde into main. Review event: COMMENT.
Estimated review effort: 1/5 (~12 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
bench/BASELINE-LEDGER.md |
modified | +14/-0 | Documentation | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 0, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Maintainer Analysis
Changed behavior:
- Adds baseline-ledger entries for six merged boundaries in the v0.23.2 security/privacy train.
- Assigns #366, #374, and #384/#391 to a future F53 re-bank discharge event and records the other listed boundaries as inert or already recorded.
Affected invariants:
- Baseline status must accurately represent whether required evidence is pending or complete.
- Privacy-transform baseline claims must remain tied to an observed transform-change count and the shipped configuration.
Evidence:
- bench/BASELINE-LEDGER.md:46 describes F53 re-bank registration as the discharge event for every OPEN row.
- bench/BASELINE-LEDGER.md:57 simultaneously labels the #384/#391 row OPEN and “verified at re-bank.”
Limitations:
- Review was limited to the supplied diff; merge SHAs, issue relationships, source-line references, benchmark behavior, and external receipts were not independently inspected or executed.
No-finding rationale: No additional defect was validated from the documentation-only diff without relying on unverified repository or GitHub state.
Risk Taxonomy
- Release regression: 1
Validation and Proof
Documentation-only changed surface; runtime execution proof is not required by default.
- recommended: Docs/readme review - All reviewed files are documentation paths. Proof: PR description notes the docs intent.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Related Context
Related issues/PRs: #364, #366, #370, #374, #384, #391, #155, #377.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
…e already discharged at rc3/GA Review thread on #412: the harness never runs LCMEngine compaction or clone startup, so 'compaction counts must not move' is unobservable for any bench row. The row now records that no bench re-baseline is owed and that the observable gate (release-gauntlet Phase C multi-clone soak) already ran post-#364 on the rc3/GA tree.
There was a problem hiding this comment.
Walkthrough
PR: #412 - bench: BASELINE-LEDGER rows for the v0.23.2 security/privacy train
Head: da352f80c029c34087b042292cb000eafdaafd5c into main. Review event: COMMENT.
Estimated review effort: 1/5 (~12 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
bench/BASELINE-LEDGER.md |
modified | +14/-0 | Documentation | Moderate: validated P2 finding |
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 1, P3: 0).
Dropped findings before posting: 0. High-severity findings: 0.
Maintainer Analysis
Changed behavior:
- Adds security/privacy-train baseline-ledger entries covering SQLite clone startup, provider-copy redaction, recall error handling, lossless defaults, truncated-PEM scanning, and a session-end refactor.
Affected invariants:
- Benchmark baselines must be re-registered when content shaping can change embedding inputs.
- Release-readiness evidence must distinguish construction-level inertness from corpus-measured inertness.
Evidence:
- bench/BASELINE-LEDGER.md:55 states both that the benchmark now invokes the provider-copy transform and that no baseline moves are possible by construction.
- bench/BASELINE-LEDGER.md:54 states that redaction may change text without blocking and therefore requires a transform-change count.
Limitations:
- Review was limited to the supplied checkout diff; no commands, tests, builds, external services, or PR code were executed.
- Referenced merge commits, receipts, issues, source line numbers, and live-soak results were not independently verified.
No-finding rationale: No additional defect was validated from the documentation-only diff; the other rows either record open evidence obligations or describe changes that are explicitly claimed to be benchmark-unreachable.
Risk Taxonomy
- Release regression: 1
Validation and Proof
Documentation-only changed surface; runtime execution proof is not required by default.
- recommended: Docs/readme review - All reviewed files are documentation paths. Proof: PR description notes the docs intent.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Related Context
Related issues/PRs: #364, #366, #370, #374, #384, #391, #155, #377.
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
|
Merge receipt — landed as |
…LOG header scope Review findings (evaos P3 + cross-model r1): the summary said three product fixes where #407 is test-only; #412 also recorded the #155 refactor row (ledger entry only) so the notes no longer claim no new ledger row; the CHANGELOG header now scopes the curated-notes claim to tags from v0.21.0-rc1 onward, which is where the notes directory actually starts.
1 participant
Summary
Append-only
bench/BASELINE-LEDGER.mdrows for the main-tree merges of the v0.23.2 train that shape recall-path content, the compaction path, or the eval instrument: #364 (read-only current-schema clones), #366 (private-key redaction ordering, privacy:v1→v3), #370 (privacy errors fail loud + harness posture), #374 (lossless by default, provider-copy protection, rerank privacy), #384 + #391 (truncated-PEM scanner, rc1/rc2 gauntlet catches), #155 (session-end mixin, AST-identical). Six rows; no existing row edited.Linked work and change class
Derivation (mechanical, per the ledger rule)
gh pr view <n> --repo electricsheephq/lcm-x --json mergeCommitfor 364/366/370/374/384/391/155 (bc03310, f21fc84, fbe24cc, c0402f3, 59bba76, 9e43907, a84ca05).origin/mainc98cb6a:ingest_protection.py:396privacy:v3;config.py:702/707flags;tools.py:3762/5457/5461protect calls;benchmarking/longmemeval.py:478/1588/1655harness transform hook.Claim class
Verified-factual ledger rows. Does NOT prove: that any OPEN row is actually inert on the LongMemEval corpus — that is exactly what the re-bank's
privacy_blocked == 0check settles.