crates/stdlib/src/csv.rs (3)

1516-1525: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Avoid a hard assert_eq! panic on a runtime buffer invariant.

If csv-core ever emits the record without the sentinel as the final byte, this panics and takes down the interpreter instead of surfacing a Python-level error. buffer_offset is also used unguarded for the - 1 index. A checked strip keeps the fast path identical and degrades safely.

🛡️ Proposed fix

-        assert_eq!(buffer[buffer_offset - 1], CSV_CORE_TERMINATOR_SENTINEL);
-        let mut output = buffer[..buffer_offset - 1].to_vec();
+        let emitted = &buffer[..buffer_offset];
+        let body = emitted
+            .strip_suffix(&[CSV_CORE_TERMINATOR_SENTINEL])
+            .ok_or_else(|| new_csv_error(vm, "internal error: missing record terminator"))?;
+        let mut output = body.to_vec();
         output.extend_from_slice(self.dialect.lineterminator.as_bytes());

🤖 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/stdlib/src/csv.rs` around lines 1516 - 1525, Replace the unchecked
sentinel assertion and buffer_offset - 1 access in the CSV serialization path
with a checked strip operation that verifies buffer_offset is nonzero and the
final byte is CSV_CORE_TERMINATOR_SENTINEL. Preserve the existing output
construction when the sentinel is present, and return the established
Python-level error through the surrounding error-handling path when the
invariant fails.

---

841-841: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

csv-core quoting decisions now ignore the real terminator bytes.

Since the builder is told the terminator is the sentinel \n, csv-core's "necessary" quoting logic will never quote a field because it contains a byte of a custom terminator (e.g. !@#). This is harmless for QUOTE_ALL, and mostly harmless for QUOTE_NONNUMERIC (strings are always quoted), but an unquoted stringified non-string field containing a terminator byte would round-trip incorrectly. Worth a follow-up test or an explicit note here.

🤖 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/stdlib/src/csv.rs` at line 841, Update the writer setup around
writer.terminator in the CSV serialization path so csv-core’s necessary-quoting
logic recognizes the actual custom terminator bytes rather than only the
sentinel newline. Preserve existing behavior for standard terminators and add a
regression test covering an unquoted stringified non-string field containing a
custom terminator byte.

---

237-250: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Use a single exception type for invalid lineterminator handling.

prase_lineterminator_from_obj rejects "lineterminator": "" with _csv.Error, while FormatOptions::from_args rejects the same condition with TypeError. This makes Python-level except behavior depend on whether the value comes from a dialect object or keyword arguments. Also, CPython allows lineterminator="" for writers, so if full CSV parity is desired, remove the empty-check from both paths.

🤖 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/stdlib/src/csv.rs` around lines 237 - 250, Make invalid lineterminator
handling consistent across prase_lineterminator_from_obj and
FormatOptions::from_args by using the same exception type for empty values. If
matching CPython writer behavior, remove the empty-string rejection from both
paths so lineterminator="" is accepted; otherwise apply the shared exception
consistently.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/stdlib/src/csv.rs (1)

1326-1470: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: factor out the shared row-writing scaffolding.

writerow_quoted_strings, writerow_quote_none, and writerow_minimal differ only in the per-field quoting decision; the ArgIterable coercion, match_class! stringification, delimiter insertion, terminator append, and UTF-8 conversion are copied three times. A single helper taking a per-field closure (or a small enum) would remove ~60 duplicated lines and keep future terminator/quoting fixes in one place.

As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code".

🤖 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/stdlib/src/csv.rs` around lines 1326 - 1470, Optionally extract the
shared row-writing flow from writerow_quoted_strings, writerow_quote_none, and
writerow_minimal into one helper, parameterizing only each mode’s per-field
quoting decision. Preserve the existing iterable conversion, field
stringification, delimiter handling, line terminator, UTF-8 conversion, and
error behavior while removing the duplicated scaffolding.

Source: Coding guidelines