Skip to content

fix(cli): parse JSONL log lines that omit or capitalize the level field - #3082

Open
phil-opp wants to merge 1 commit into
mainfrom
claude/dreamy-bardeen-v852wi-jsonl-missing-level
Open

fix(cli): parse JSONL log lines that omit or capitalize the level field#3082
phil-opp wants to merge 1 commit into
mainfrom
claude/dreamy-bardeen-v852wi-jsonl-missing-level

Conversation

@phil-opp

@phil-opp phil-opp commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Issue

parse_jsonl_line (binaries/cli/src/output.rs) parses daemon/compact JSONL log lines and is documented to tolerate compact formats. It had two related problems in its level handling:

let level_str = v.get("level")?.as_str().unwrap_or("stdout");
let level = match level_str {
    "error" => ..., "warn" => ..., "info" => ..., "debug" => ..., "trace" => ...,
    _ => LogLevelOrStdout::Stdout,
};
  1. Missing key drops the line. The ? on v.get("level") returns None — dropping the entire line — whenever the level key is absent. The .unwrap_or("stdout") fallback was therefore unreachable for the common "no level key" case, so any externally-produced JSONL line with ts/msg but no level was silently discarded.
  2. Case-sensitive misclassification. The inline match compared only lowercase literals and duplicated parse_log_level_str (the single source of truth for level names, which lowercases first). An external tool emitting a capitalized level like "INFO" hit the _ => Stdout arm — the line was shown but misclassified as stdout, breaking --log-level filtering and coloring.

Fix

Look the level up through parse_log_level_str, defaulting to stdout when the key is missing, non-string, or unrecognized:

let level = v
    .get("level")
    .and_then(|l| l.as_str())
    .and_then(|s| parse_log_level_str(s).ok())
    .unwrap_or(LogLevelOrStdout::Stdout);

A line without a level is now kept (as stdout) instead of dropped, a capitalized level is classified correctly, and the duplicated match is removed.

Validation

  • Added regression tests for a missing key, a non-string value, a capitalized level ("INFO" → Info), and an unknown level.
  • cargo test -p dora-cli --lib output:: — 21 passed.
  • cargo fmt --all -- --check and cargo clippy -p dora-cli -- -D warnings clean.

The capitalization fix and the parse_log_level_str reuse were added after an automated /review + /simplify pass flagged the case-sensitive duplication.


🤖 This is a machine-generated pull request opened by Claude Code as part of an automated code-review pass. Please review carefully before merging.

🤖 Generated with Claude Code

@trunk-io

trunk-io Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

`parse_jsonl_line` claimed to default a missing level to stdout
(`.unwrap_or("stdout")`), but the preceding `v.get("level")?` returned
`None` and discarded the whole line whenever the `level` key was absent,
so the fallback was unreachable for the common "no level key" case.

Additionally, the inline level match was case-sensitive and duplicated
`parse_log_level_str` (the single source of truth for level names): an
external tool emitting a capitalized level like "INFO" was shown but
misclassified as stdout, breaking level filtering and coloring.

Look the level up with `parse_log_level_str` and default to stdout when
the key is missing, non-string, or unrecognized. A line without a level
is now kept (as stdout) instead of dropped, and a capitalized level is
classified correctly. Adds regression tests for a missing key, a
non-string value, a capitalized level, and an unknown level.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SD4dBVimzepwKSh2b9F8aG
@phil-opp
phil-opp force-pushed the claude/dreamy-bardeen-v852wi-jsonl-missing-level branch from 1177200 to 2997711 Compare August 9, 2026 00:14
@phil-opp phil-opp changed the title fix(cli): don't drop JSONL log lines that omit the level field fix(cli): parse JSONL log lines that omit or capitalize the level field Aug 9, 2026

phil-opp commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Automated review by Claude — this is a fully automated review with no human in the loop.

I reviewed this diff and found no issues. The fix correctly replaces v.get("level")? (which dropped the whole line when level was absent) with v.get("level").and_then(as_str).and_then(parse_log_level_str).unwrap_or(Stdout), so a missing / non-string / unknown level now defaults to stdout instead of discarding the line, and delegating to parse_log_level_str (which lowercases) classifies capitalized levels correctly. The four regression tests are meaningful and each fails against the old code.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants