fix(output): escape compact CSV fields in compact console output#73
Conversation
|
CodeAnt AI is reviewing your PR. |
📝 WalkthroughWalkthroughCentralized CSV escaping into Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
codeant-ai
Bot
added
the
size:M
User descriptionCloses #70. I tripped over this while checking compact output for passphrases that include punctuation. A comma in This change makes compact output serialize each field with RFC 4180-style escaping instead of raw string interpolation. It also moves CSV field escaping into a shared helper so query CSV and compact console output use the same rules. What changed:
Validation run:
Note: full CodeAnt-AI DescriptionEscape CSV fields in compact console output and add tests What Changed
Impact
💡 Usage GuideChecking Your Pull RequestEvery time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later. Talking to CodeAnt AIGot a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask: This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code. ExamplePreserve Org Learnings with CodeAntYou can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input: This helps CodeAnt AI learn and adapt to your team's coding style and standards. ExampleRetrigger reviewAsk CodeAnt AI to review the PR again, by typing: Check Your Repository HealthTo analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health. |
Sequence DiagramThis PR centralizes CSV field escaping so both compact console key output and storage query CSV output use the same RFC 4180 style rules, preventing misaligned columns when fields contain commas, quotes, or newlines. sequenceDiagram
participant User
participant CLI
participant ConsoleOutput
participant QueryCsvOutput
participant CsvHelper
participant Destination
User->>CLI: Run command with CSV style output
par Compact console output
CLI->>ConsoleOutput: Output derived key in compact format
ConsoleOutput->>CsvHelper: Escape compact CSV fields
CsvHelper-->>ConsoleOutput: Escaped CSV row
ConsoleOutput->>Destination: Write compact CSV row
and Query CSV output
CLI->>QueryCsvOutput: Render query result as CSV
QueryCsvOutput->>CsvHelper: Escape query CSV fields
CsvHelper-->>QueryCsvOutput: Escaped CSV row
QueryCsvOutput->>Destination: Write CSV row
end
Generated by CodeAnt AI |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Isolated fix for CSV escaping in output formatting. Extracted a shared helper and added comprehensive tests for edge cases. Low risk and improves data integrity of output.
Architecture diagram
sequenceDiagram
participant App as Application Logic
participant CO as ConsoleOutput (console.rs)
participant QF as QueryFormat (query_format.rs)
participant Utils as Output Helpers (mod.rs)
participant IO as Stdout / File System
Note over App, IO: Compact Console Output Flow
App->>CO: key(source, transform, derived_key)
CO->>CO: NEW: format_compact_csv_row(...)
loop For each field (source, transform, etc.)
CO->>Utils: NEW: escape_csv_field(field)
alt field contains [ , " \n \r ]
Utils->>Utils: RFC 4180: Wrap in quotes & escape internal quotes
else simple string
Utils->>Utils: Return raw string
end
Utils-->>CO: escaped_field
end
CO->>CO: Join fields with ","
CO->>IO: CHANGED: writeln!(writer, escaped_row)
Note over App, IO: Query Results Flow (CSV)
App->>QF: format_value_csv(value)
QF->>Utils: CHANGED: escape_csv_field(field)
Note right of Utils: Logic moved from QueryFormat to shared Utils
Utils-->>QF: escaped_field
QF-->>App: formatted CSV cell
🤖 Augment PR SummarySummary: Fixes compact console output so it emits valid CSV when fields contain commas/quotes/newlines, preventing downstream column misalignment. Changes:
Technical Notes: Escaping wraps fields containing 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/output/console.rs`:
- Around line 185-225: Replace the loose substring assertions in the three tests
(test_to_file_writes_compact_format_with_commas_in_source,
test_to_file_writes_compact_format_with_quotes_in_source,
test_to_file_writes_compact_format_with_newlines_in_source) so they assert the
exact file contents produced by ConsoleOutput::to_file and the key(...) calls;
read the file into content and compare it for equality to the full expected CSV
row string (including newline/end-of-file) instead of using
content.contains(...), ensuring the expected strings match the escaping rules
used by key(...) and make_test_key().
- Around line 185-225: Change the three test functions
(test_to_file_writes_compact_format_with_commas_in_source,
test_to_file_writes_compact_format_with_quotes_in_source,
test_to_file_writes_compact_format_with_newlines_in_source) to return Result<()>
and replace all .unwrap() calls in setup and I/O with the ? operator (e.g.
NamedTempFile::new()?; ConsoleOutput::to_file(temp.path())?; output.key(...)?;
output.flush()?; let content = std::fs::read_to_string(temp.path())?;), keeping
assert! checks unchanged; this makes failures surface as returned errors instead
of panics while preserving use of ConsoleOutput::to_file and make_test_key.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: a925e48a-7394-4fdd-8254-85cd83f126ca
📒 Files selected for processing (3)
src/output/console.rssrc/output/mod.rssrc/output/query_format.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: benchmarks
🧰 Additional context used
📓 Path-based instructions (3)
src/{transform,analyze,source,output,storage}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
All core trait implementations must satisfy
Send + Syncbounds
Files:
src/output/mod.rssrc/output/console.rssrc/output/query_format.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Prefer?operator over.unwrap()for error handling in Rust code
Avoidpanic!()macro; use Result types for error handling instead
Never use type suppression or type-unsafe casting patterns (equivalent toas anyor@ts-ignore)
Files:
src/output/mod.rssrc/output/console.rssrc/output/query_format.rs
src/output/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Implement the Output trait for new output formats in dedicated
src/output/{name}.rsfiles
Files:
src/output/mod.rssrc/output/console.rssrc/output/query_format.rs
🧠 Learnings (9)
📚 Learning: 2026-03-02T14:18:44.316Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/transform/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:44.316Z
Learning: Applies to src/transform/*.rs : Process inputs as batch operations using `&[Input]` as input and `&mut Vec<(String, Key)>` as output, where the first tuple element is a human-readable source description
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/{transform,analyze}/**/*.rs : Always process inputs and outputs as batches: `&[Input]` → `&mut Vec<(String, Key)>`
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/output/**/*.rs : Implement the Output trait for new output formats in dedicated `src/output/{name}.rs` files
Applied to files:
src/output/console.rssrc/output/query_format.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/derive.rs : Key derivation logic including secp256k1 and Bitcoin address/WIF generation must be implemented in `src/derive.rs`
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-05T12:48:44.245Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/source/AGENTS.md:0-0
Timestamp: 2026-03-05T12:48:44.245Z
Learning: Applies to src/source/!(mod).rs : Implement `Source` trait with `process()` method accepting `transforms`, `deriver`, `matcher`, and `output` parameters
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/transform/**/*.rs : Organize transforms in `src/transform/{name}.rs` modules with clear separation between implementation and trait definition
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:08.405Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/analyze/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:08.405Z
Learning: Applies to src/analyze/analyze/output.rs : Format analysis results as plain text and JSON using utilities in `output.rs`
Applied to files:
src/output/console.rssrc/output/query_format.rs
📚 Learning: 2026-03-02T14:18:34.668Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/storage/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:34.668Z
Learning: Applies to src/storage/storage/schema.rs : Update Arrow schema definition in `schema.rs`, including `result_schema()` function and field constants
Applied to files:
src/output/query_format.rs
📚 Learning: 2026-03-02T14:18:34.668Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/storage/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:34.668Z
Learning: Applies to src/storage/storage/schema.rs : Map variable-length fields to fixed columns in schema for SQL-friendly querying
Applied to files:
src/output/query_format.rs
🧬 Code graph analysis (2)
src/output/console.rs (1)
src/output/mod.rs (1)
escape_csv_field(21-27)
src/output/query_format.rs (1)
src/output/mod.rs (1)
escape_csv_field(21-27)
🔇 Additional comments (4)
src/output/mod.rs (1)
21-27: Centralizing CSV escaping here fixes the drift problem.Both query CSV output and compact console output now run through the same rules, so commas, quotes, and embedded newlines stop diverging between formats.
src/output/console.rs (1)
9-9: Escaping each field before join is the right fix here.The old failure path was raw interpolation into a comma-separated line. Building the row from escaped fields closes that path for every compact column, not just
source.Also applies to: 79-85, 130-141
src/output/query_format.rs (2)
8-8: Reusing the shared escape helper here is the right move.This keeps query CSV output and compact console output on one escaping implementation, which is exactly where these bugs tend to drift apart.
266-273: No review comment.Assertion wrapping only. No behavior change in this segment.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/output/console.rs (2)
186-224:⚠️ Potential issue | 🟡 MinorReturn
Result<()>from the new tests and use?.These tests do file creation and I/O.
unwrap()turns setup failures into panics and hides which step failed.Apply the same pattern to all three new tests
- fn test_to_file_writes_compact_format_with_commas_in_source() { - let temp = NamedTempFile::new().unwrap(); - let output = ConsoleOutput::to_file(temp.path()).unwrap(); + fn test_to_file_writes_compact_format_with_commas_in_source() -> Result<()> { + let temp = NamedTempFile::new()?; + let output = ConsoleOutput::to_file(temp.path())?; output .key("test,source", "sha256", &make_test_key()) - .unwrap(); - output.flush().unwrap(); + ?; + output.flush()?; - let content = std::fs::read_to_string(temp.path()).unwrap(); + let content = std::fs::read_to_string(temp.path())?; assert_eq!(content, "\"test,source\",sha256,abc123,1Address\n"); + Ok(()) }As per coding guidelines:
src/**/*.rs: Prefer?operator over.unwrap()for error handling in Rust code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/output/console.rs` around lines 186 - 224, Change the three test functions (test_to_file_writes_compact_format_with_commas_in_source, test_to_file_writes_compact_format_with_quotes_in_source, test_to_file_writes_compact_format_with_newlines_in_source) to return Result<(), Box<dyn std::error::Error>> (or anyhow::Result<()>) and replace all .unwrap() calls used for setup and I/O with the ? operator; keep the assertions as-is and add a final Ok(()) return so failures propagate as test errors instead of panics.
195-196:⚠️ Potential issue | 🟡 MinorAssert the full row in these regression tests.
Line 196 and Line 210 still use
contains(...), so the tests pass if compact output grows an extra column or extra text around the row. That misses the CSV-shape regression this PR is fixing.Suggested tightening
- assert!(content.contains("\"test,source\",sha256,abc123,1Address")); + assert_eq!(content, "\"test,source\",sha256,abc123,1Address\n");- assert!(content.contains("\"say \"\"hello\"\"\",sha256,abc123,1Address")); + assert_eq!(content, "\"say \"\"hello\"\"\",sha256,abc123,1Address\n");Also applies to: 209-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/output/console.rs` around lines 195 - 196, The test reads the temp file into content and currently uses assert!(content.contains("...")) which allows extra columns; change the assertions that reference content.contains(...) (e.g., the one checking "\"test,source\",sha256,abc123,1Address" and the duplicate at lines around 209–210) to assert the exact CSV row instead — for example split content.lines() and compare the specific line to the exact expected string with assert_eq! or trim_end_matches to include the newline, so the test fails if additional columns or surrounding text are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/output/console.rs`:
- Around line 186-224: Change the three test functions
(test_to_file_writes_compact_format_with_commas_in_source,
test_to_file_writes_compact_format_with_quotes_in_source,
test_to_file_writes_compact_format_with_newlines_in_source) to return Result<(),
Box<dyn std::error::Error>> (or anyhow::Result<()>) and replace all .unwrap()
calls used for setup and I/O with the ? operator; keep the assertions as-is and
add a final Ok(()) return so failures propagate as test errors instead of
panics.
- Around line 195-196: The test reads the temp file into content and currently
uses assert!(content.contains("...")) which allows extra columns; change the
assertions that reference content.contains(...) (e.g., the one checking
"\"test,source\",sha256,abc123,1Address" and the duplicate at lines around
209–210) to assert the exact CSV row instead — for example split content.lines()
and compare the specific line to the exact expected string with assert_eq! or
trim_end_matches to include the newline, so the test fails if additional columns
or surrounding text are present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c71165c-c464-477a-80e3-bee10f849db5
📒 Files selected for processing (3)
src/output/console.rssrc/output/mod.rssrc/output/query_format.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: benchmarks
🧰 Additional context used
📓 Path-based instructions (3)
src/{transform,analyze,source,output,storage}/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
All core trait implementations must satisfy
Send + Syncbounds
Files:
src/output/console.rssrc/output/query_format.rssrc/output/mod.rs
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.rs: Prefer?operator over.unwrap()for error handling in Rust code
Avoidpanic!()macro; use Result types for error handling instead
Never use type suppression or type-unsafe casting patterns (equivalent toas anyor@ts-ignore)
Files:
src/output/console.rssrc/output/query_format.rssrc/output/mod.rs
src/output/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Implement the Output trait for new output formats in dedicated
src/output/{name}.rsfiles
Files:
src/output/console.rssrc/output/query_format.rssrc/output/mod.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/output/**/*.rs : Implement the Output trait for new output formats in dedicated `src/output/{name}.rs` files
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/analyze/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:08.405Z
Learning: Applies to src/analyze/analyze/output.rs : Format analysis results as plain text and JSON using utilities in `output.rs`
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/output/**/*.rs : Implement the Output trait for new output formats in dedicated `src/output/{name}.rs` files
Applied to files:
src/output/console.rssrc/output/query_format.rs
📚 Learning: 2026-03-02T14:18:08.405Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/analyze/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:08.405Z
Learning: Applies to src/analyze/analyze/output.rs : Format analysis results as plain text and JSON using utilities in `output.rs`
Applied to files:
src/output/console.rssrc/output/query_format.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/**/*.rs : Avoid `panic!()` macro; use Result types for error handling instead
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:44.316Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/transform/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:44.316Z
Learning: Applies to src/transform/*.rs : Process inputs as batch operations using `&[Input]` as input and `&mut Vec<(String, Key)>` as output, where the first tuple element is a human-readable source description
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/{transform,analyze}/**/*.rs : Always process inputs and outputs as batches: `&[Input]` → `&mut Vec<(String, Key)>`
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/derive.rs : Key derivation logic including secp256k1 and Bitcoin address/WIF generation must be implemented in `src/derive.rs`
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-05T12:48:44.245Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/source/AGENTS.md:0-0
Timestamp: 2026-03-05T12:48:44.245Z
Learning: Applies to src/source/!(mod).rs : Implement `Source` trait with `process()` method accepting `transforms`, `deriver`, `matcher`, and `output` parameters
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:00.377Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:00.377Z
Learning: Applies to src/transform/**/*.rs : Organize transforms in `src/transform/{name}.rs` modules with clear separation between implementation and trait definition
Applied to files:
src/output/console.rs
📚 Learning: 2026-03-02T14:18:34.668Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/storage/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:34.668Z
Learning: Applies to src/storage/storage/schema.rs : Update Arrow schema definition in `schema.rs`, including `result_schema()` function and field constants
Applied to files:
src/output/query_format.rs
📚 Learning: 2026-03-02T14:18:34.668Z
Learnt from: CR
Repo: oritwoen/vuke PR: 0
File: src/storage/AGENTS.md:0-0
Timestamp: 2026-03-02T14:18:34.668Z
Learning: Applies to src/storage/storage/schema.rs : Map variable-length fields to fixed columns in schema for SQL-friendly querying
Applied to files:
src/output/query_format.rs
🧬 Code graph analysis (2)
src/output/console.rs (1)
src/output/mod.rs (1)
escape_csv_field(21-35)
src/output/query_format.rs (1)
src/output/mod.rs (1)
escape_csv_field(21-35)
1 participant
Closes #70.
I tripped over this while checking compact output for passphrases that include punctuation. A comma in
sourceshifted columns, so downstream parsing got misaligned.This change makes compact output serialize each field with RFC 4180-style escaping instead of raw string interpolation. It also moves CSV field escaping into a shared helper so query CSV and compact console output use the same rules.
What changed:
ConsoleOutput::keyescape_csv_fieldtosrc/output/mod.rsand reused it fromquery_format.rssourceValidation run:
cargo test --lib output::consolecargo test --features storage-query output::query_formatcargo buildcubic review -p \"Focus on real bugs, security issues, logic errors, type safety problems, and missed edge cases. Skip cosmetic nits and formatting that linters handle. Be direct and specific: what is wrong, where, and why it matters. No filler, no praise, no generic advice.\"coderabbit review --plain --type uncommittedNote: full
cargo testcurrently has one pre-existing unrelated failure inanalyze::lcg::tests::test_masked_analysis.