feat(provider): import Diaphora binary-diff evidence - #252
Conversation
|
✅ Health of changed files: 5.3 → 6.1 (+0.7) 📋 At a glance ✅ Health gate: passed 📌 Before you merge
🔎 More signals (3)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (1 with dependents)"]
f_crates_code_intel_cli_src_main_rs[".../src/main.rs 🔥"]:::changed
end
f_crates_code_intel_cli_src_change_risk_git_rs[".../change_risk/git.rs"]
f_crates_code_intel_cli_src_main_rs --> f_crates_code_intel_cli_src_change_risk_git_rs
f_crates_code_intel_cli_src_doctor_adapter_rs[".../src/doctor_adapter.rs"]
f_crates_code_intel_cli_src_main_rs --> f_crates_code_intel_cli_src_doctor_adapter_rs
w_orchestration_integrations_json(["⚠️ orchestration/integrations.json changed together 19×, not in PR"]):::warn
f_crates_code_intel_cli_src_main_rs -.- w_orchestration_integrations_json
w_orchestration_internalization_sentrux_json(["⚠️ orchestration/internalization/sentrux.json changed together 12×, not in PR"]):::warn
f_crates_code_intel_cli_src_main_rs -.- w_orchestration_internalization_sentrux_json
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (2 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (4)
1 more
🔗 Hidden coupling (1 file)
📊 See the full report for this PR |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an optional, read-only Diaphora provider. The CLI validates SQLite result databases, hashes inputs, emits bounded observation artifacts, and reports unavailable or rejected states. Route metadata, orchestration integration, schema contracts, tests, help output, and documentation are included. ChangesDiaphora provider integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant diaphora_provider
participant SQLiteDatabase
participant ObservationArtifact
CLI->>diaphora_provider: run_raw arguments
diaphora_provider->>SQLiteDatabase: open read-only and inspect schema
SQLiteDatabase-->>diaphora_provider: configuration, matches, and unmatched results
diaphora_provider->>ObservationArtifact: serialize observation with hashes and summaries
ObservationArtifact-->>CLI: write and print JSON
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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: 1
🤖 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 `@crates/code-intel-cli/src/diaphora_provider.rs`:
- Around line 143-151: Update inspect so result_database_sha256 is computed from
the same stable SQLite state that the read-only Connection inspects: either
reject journal/WAL sidecars before hashing and opening, or create a stable
snapshot, hash it, and inspect that snapshot. Ensure the emitted rows remain
cryptographically bound to result_database_sha256 while preserving the existing
base and candidate hashing.
🪄 Autofix
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 Plus
Run ID: e2bdcbcb-fbe8-4e43-ad38-2d6237bd65d2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
crates/code-intel-cli/Cargo.tomlcrates/code-intel-cli/src/cli/command_catalog/mod.rscrates/code-intel-cli/src/cli/command_catalog/routes/mod.rscrates/code-intel-cli/src/cli/command_catalog/routes/provider_routes.rscrates/code-intel-cli/src/cli/command_catalog/tests.rscrates/code-intel-cli/src/cli/legacy.rscrates/code-intel-cli/src/diaphora_provider.rscrates/code-intel-cli/src/main.rscrates/code-intel-cli/tests/diaphora_provider.rscrates/code-intel-cli/tests/fixtures/cli-head-parity.v2.jsondocs/diaphora-provider-adapter.mdorchestration/integrations.jsonorchestration/schemas/code-intel-diaphora-observation.v1.schema.json
| fn inspect(cli: &Cli) -> Result<Value, InspectionError> { | ||
| let base_binary_sha256 = file_sha256(&cli.base_binary, "base binary")?; | ||
| let candidate_binary_sha256 = file_sha256(&cli.candidate_binary, "candidate binary")?; | ||
| let result_database_sha256 = file_sha256(&cli.result_db, "Diaphora result database")?; | ||
| let connection = Connection::open_with_flags( | ||
| &cli.result_db, | ||
| OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX, | ||
| ) | ||
| .map_err(|_| InspectionError::Rejected("Diaphora result database is unreadable".into()))?; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files around target =="
git ls-files | rg '(^|/)diaphora_provider\.rs$|crates/code-intel-cli/src|Cargo\.toml'
echo "== target file outline =="
ast-grep outline crates/code-intel-cli/src/diaphora_provider.rs --view expanded || true
echo "== target file relevant lines =="
nl -ba crates/code-intel-cli/src/diaphora_provider.rs | sed -n '1,260p'
echo "== search for resultDatabaseSha256 and related hashes =="
rg -n "resultDatabaseSha256|base_binary_sha256|candidate_binary_sha256|file_sha256|result_db|wal|WAL|journal" crates/code-intel-cli/src -SRepository: 2233admin/code-intel-pipeline
Length of output: 9824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files mentioning Diaphora sqlite inspection =="
rg -n "Diaphora|diaphora|sqlite|Sqlite|OpenFlags|SQLITE_OPEN" -S .
echo "== tests for inspect/result database =="
rg -n "#\\[test\\]|inspections|Inspect|Inspection|result database|result_db|wal|WAL" crates/code-intel-cli/src tests integration-test -S 2>/dev/null || trueRepository: 2233admin/code-intel-pipeline
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file relevant sections =="
sed -n '130,215p' crates/code-intel-cli/src/diaphora_provider.rs
echo "== tail =="
sed -n '320,435p' crates/code-intel-cli/src/diaphora_provider.rs
echo "== tests relevant sections =="
sed -n '1,260p' crates/code-intel-cli/tests/diaphora_provider.rs
echo "== schema relevant fields =="
sed -n '1,130p' orchestration/schemas/code-intel-diaphora-observation.v1.schema.json
echo "== docs relevant sections =="
sed -n '1,55p' docs/diaphora-provider-adapter.mdRepository: 2233admin/code-intel-pipeline
Length of output: 22172
🌐 Web query:
SQLite WAL mode read transactions read-only connection includes WAL file
💡 Result:
In SQLite WAL (Write-Ahead Logging) mode, a read-only database connection can indeed read from the WAL file, and this is a core part of how WAL mode enables concurrent reading and writing [1][2]. When a connection performs a read transaction in WAL mode, it does not exclusively rely on the main database file. Instead, it uses the WAL file and the associated shared memory (-shm) file to access a consistent "snapshot" of the database as it existed when the read transaction began [1][2][3]. If a requested page has been modified by a committed transaction, the reader retrieves the most recent version of that page from the WAL file rather than the main database file [1]. Historically, all connections to a WAL-mode database—including read-only ones—required write permission to the directory containing the database to create or access the -shm and -wal files [1][4]. However, as of SQLite version 3.22.0, this constraint has been relaxed [1]. A read-only process can now access a WAL-mode database without needing write permissions, provided one of the following conditions is met [1]: 1. The -shm and -wal files already exist and are readable by the process [1]. 2. The database connection is opened using the "immutable" query parameter (e.g., in a URI filename), which signals that the database is on read-only media and cannot change, thereby bypassing locking and change detection [1][5][6]. 3. The process has write permission on the directory to create the -shm and -wal files if they are missing [1]. If a database is intended to be used on read-only media, it is generally recommended to convert it to a different journal mode (such as journal_mode=DELETE) to avoid the dependencies and potential file-access issues associated with WAL files and -shm files [1][7].
Citations:
- 1: https://www.sqlite.org/wal.html
- 2: https://www.sqlite.org/isolation.html
- 3: https://github.com/sqlite/sqlite/blob/master/src/wal.c
- 4: https://sqlite.org/forum/forumpost/184d4baa2c244b19
- 5: https://sqlite.org/c3ref/open.html
- 6: https://www.sqlite.org/uri.html
- 7: https://sqlite.org/forum/forumpost/9e90fcff37d42591
Use one stable digest for the inspected SQLite snapshot.
resultDatabaseSha256 only hashes cli.result_db before SQLite opens it. A WAL-backed database can expose -wal contents through this read-only connection while that digest does not cover them. Reject journal/WAL sidecars, or take one stable SQLite snapshot and hash the snapshot before inspection so resultDatabaseSha256 actually binds the emitted rows.
🤖 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 `@crates/code-intel-cli/src/diaphora_provider.rs` around lines 143 - 151,
Update inspect so result_database_sha256 is computed from the same stable SQLite
state that the read-only Connection inspects: either reject journal/WAL sidecars
before hashing and opening, or create a stable snapshot, hash it, and inspect
that snapshot. Ensure the emitted rows remain cryptographically bound to
result_database_sha256 while preserving the existing base and candidate hashing.
Code Intel change risk
Top signals
revspec: |
1 participant
Closes #251
Summary
Verification