Skip to content

fix(logging): send library logs to stderr and give failures a real se… - #781

Open
Ayushrawatcy009 wants to merge 1 commit into
Ryan-Millard:devfrom
Ayushrawatcy009:fix/727-logging-stderr-severity
Open

Ayushrawatcy009 wants to merge 1 commit into
Ryan-Millard:devfrom
Ayushrawatcy009:fix/727-logging-stderr-severity

Conversation

@Ayushrawatcy009

Copy link
Copy Markdown
Contributor

…verity

Changes & Reason

Changes

  • log.h: the logger now uses spdlog::stderr_color_mt instead of the stdout one.
  • gpu.h: gave each log call the level from the table in the issue:
    • ERROR: Shader Error, [WEBGPU FATAL ERROR], [DEVICE LOST]
    • WARN: all the "Falling back to CPU" lines, Adapter Failed, Device Failed
    • INFO: Adapter Acquired, Device Acquired, GPU Fully Initialized (left as is)
    • DEBUG: Requesting Adapter/Device and the two limits lines
  • kmeans_gpu.cpp and bilateral_filter_gpu.cpp: all the step tracing is now DEBUG instead of INFO.

I didn't add any new log statements or touch what IMG2NUM_LOG_LEVEL=AUTO resolves to. Release builds aren't affected since logging is compiled out there.

Reason

The logger was writing to stdout, so any program that prints its result there (like piping an SVG into another command) got log lines mixed into the output. On top of that everything was INFO, so building with IMG2NUM_LOG_LEVEL=WARN hid the real failures, and INFO was too noisy to keep on.

Related Issues

Fixes: #727

Testing & Verification

  • just cmake build Debug, then ran the C++ console example with > out.txt 2> err.txt. out.txt had 0 log lines and err.txt had all 35 (screenshot below).
  • The levels in err.txt match the table: tracing and the limits lines are [debug], and only Adapter Acquired, Device Acquired and GPU Fully Initialized are [info].
  • just cmake build Debug WARN printed 0 log lines. My GPU works, so there were no fallbacks or errors to show, but all the INFO/DEBUG noise is gone.
  • Ran the format step and checked that only these 4 files changed.

WASM build not tested. I don't have Emscripten installed, so I couldn't do the just js build Debug check from the issue. Since Emscripten sends stderr to console.error, the logs might show up as errors in the browser console. If you see that, I can add the #ifdef __EMSCRIPTEN__ fallback to stdout in this PR, or as a follow-up, whichever you prefer.

Additional Resources

Debug build, stdout vs stderr:
image

WARN build:
image

Heads up: this touches lines near "No usable WebGPU adapter found" in gpu.h, which the clang-format PR also touches, so whichever lands second needs a rebase.

Copilot AI balanced review requested due to automatic review settings October 17, 2026 10:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Ryan-Millard/Img2Num/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 28be8d33-acfc-4b20-bf61-38290bcbe4c5
📥 Commits

Reviewing files that changed from the base of the PR and between aa801b8 and ec97455.

📒 Files selected for processing: 4
  • core/include/internal/gpu.h
  • core/include/internal/log.h
  • core/src/internal/bilateral_filter_gpu.cpp
  • core/src/internal/kmeans_gpu.cpp

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📜 Recent review details
🧰 Additional context used
📚 Code guidelines: 1
.editorconfig — unknown
📓 Path-based instructions: 4
This is the Img2Num core C/C++ library.

⚙️ CodeRabbit configuration file

Files:

  • core/include/internal/log.h
  • core/src/internal/bilateral_filter_gpu.cpp
  • core/src/internal/kmeans_gpu.cpp
  • core/include/internal/gpu.h
For any C/C++ code outside core/ (e.g.

⚙️ CodeRabbit configuration file

Files:

  • core/include/internal/log.h
  • core/src/internal/bilateral_filter_gpu.cpp
  • core/src/internal/kmeans_gpu.cpp
  • core/include/internal/gpu.h
Source excerpt: [*.{hpp,cpp,c,h}] indent_style = space indent_size = 4 charset = utf-8 end_of_line = lf trim_trailing_whitespace = true insert_final_newline = true max_line_length = 120

📄 CodeRabbit inference engine (.editorconfig)

Files:

  • core/include/internal/log.h
  • core/src/internal/bilateral_filter_gpu.cpp
  • core/src/internal/kmeans_gpu.cpp
  • core/include/internal/gpu.h
Source excerpt: [*] charset = utf-8 end_of_line = lf indent_style = space indent_size = 2 trim_trailing_whitespace = true insert_final_newline = true max_line_length = 120

📄 CodeRabbit inference engine (.editorconfig)

Files:

  • core/include/internal/log.h
  • core/src/internal/bilateral_filter_gpu.cpp
  • core/src/internal/kmeans_gpu.cpp
  • core/include/internal/gpu.h
🔇 Additional comments: 4
core/include/internal/gpu.h: 1

130-130: LGTM!

Also applies to: 152-152, 160-160, 171-171, 188-188, 194-194, 202-202, 213-213, 227-227, 242-243, 258-258, 277-277, 283-283

core/include/internal/log.h: 1

11-11: 🎯 Functional Correctness

stderr_color_mt does send logs to stderr, but Emscripten copies both stdout and stderr to the browser console by default and uses the same output function for both. The available contract does not support the claim that routine stderr logs appear as browser errors. Retaining stdout for Emscripten would not change their browser-console visibility.

core/src/internal/bilateral_filter_gpu.cpp: 1

59-59: LGTM!

Also applies to: 67-67, 70-70, 86-86, 172-172, 177-177

core/src/internal/kmeans_gpu.cpp: 1

251-251: LGTM!

Also applies to: 263-263, 380-380, 391-391, 398-398, 401-401, 412-412, 448-448

  • Send core logs to stderr through spdlog::stderr_color_mt.
  • Assign ERROR, WARN, INFO, and DEBUG levels to existing GPU diagnostics and progress messages.
  • The reported Debug check placed all 35 log lines on stderr and none on stdout. The reported WARN check produced no log lines because no GPU fallback or error occurred.
  • The WASM build was not tested.
Contributing author Lines added Lines removed
Not established by the available evidence 29 29

Walkthrough

The logger now writes to stderr. GPU initialization failures use WARN or ERROR, while setup progress and GPU processing-stage messages use DEBUG. GPU initialization and processing behavior remain unchanged.

Changes

GPU logging

Layer / File(s) Summary
Logger and GPU initialization messages
core/include/internal/log.h, core/include/internal/gpu.h
The logger uses a colorized stderr sink. GPU initialization progress uses DEBUG, and failure messages use WARN or ERROR.
Bilateral filter progress messages
core/src/internal/bilateral_filter_gpu.cpp
Processing-stage and completion messages use DEBUG instead of INFO.
K-Means progress messages
core/src/internal/kmeans_gpu.cpp
Progress messages use DEBUG instead of INFO.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: prachi-gupta2808

Merge Risk: ⚪ Minimal · up to ec974

The change redirects core logs to stderr and reduces routine GPU progress output. The unperformed Emscripten browser check is not a required project gate, and no concrete merge-blocking behavior is established.

Pre-merge checks: Passed 7 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 7 passed
Check name Status Explanation
Description check Passed The description clearly explains the logging destination changes, severity changes, affected files, rationale, testing, and known WASM testing limitation.
Title check Passed The title uses the Conventional Commit prefix fix(logging): and clearly describes sending library logs to stderr and assigning meaningful failure severity.
Linked Issues check Passed The changes satisfy the coding requirements in issue #727. log.h uses spdlog::stderr_color_mt. gpu.h assigns ERROR to shader, uncaptured-error, and device-lost messages; WARN to initialization f…
Out of Scope Changes check Passed The whole-PR diff changes only the four files named by issue #727: core/include/internal/log.h, core/include/internal/gpu.h, core/src/internal/bilateral_filter_gpu.cpp, and `core/src/internal/km…
No Ai Slop Pr Description Passed The description is specific and substantive. It names the four changed files, identifies stderr_color_mt, maps the GPU log messages to exact severity levels, explains the stdout-pollution problem, a…
No Strangely-Named Root Markdown Files Passed The pull-request diff adds no files at the repository root. It modifies only four existing source files, so it does not introduce an unconventional root Markdown file.
Coderabbit Config Needs Update Passed No config update is required. The PR modifies only existing .h and .cpp files, and .coderabbit.yaml already covers both extensions under reviews.path_instructions and C/C++ tools. It adds no l…
Full details: Linked Issues check

Explanation

The changes satisfy the coding requirements in issue #727. log.h uses spdlog::stderr_color_mt. gpu.h assigns ERROR to shader, uncaptured-error, and device-lost messages; WARN to initialization failures, adapter/device failures, and CPU fallbacks; INFO to successful acquisition and initialization; and DEBUG to requests and limits. GPU tracing in bilateral_filter_gpu.cpp and kmeans_gpu.cpp uses DEBUG. The diff adds no log statements and does not change IMG2NUM_LOG_LEVEL=AUTO. The requested WASM check is manual verification, so the reported lack of an Emscripten build does not create a coding failure.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff changes only the four files named by issue #727: core/include/internal/log.h, core/include/internal/gpu.h, core/src/internal/bilateral_filter_gpu.cpp, and core/src/internal/kmeans_gpu.cpp. The changes only route existing logs and adjust their levels. The diff does not add log statements, change AUTO resolution, or modify the Debug Logging documentation.

Full details: No Ai Slop Pr Description

Explanation

The description is specific and substantive. It names the four changed files, identifies stderr_color_mt, maps the GPU log messages to exact severity levels, explains the stdout-pollution problem, and records build and stream-separation checks. The HTML-comment contribution notice is standard template metadata, not evidence of AI slop.

Full details: Coderabbit Config Needs Update

Explanation

No config update is required. The PR modifies only existing .h and .cpp files, and .coderabbit.yaml already covers both extensions under reviews.path_instructions and C/C++ tools. It adds no language, linter or formatter configuration, CI configuration, or uncovered extension.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
✨ Simplify code
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit checks the logs at night
Now stderr holds each line just right
GPU steps whisper, DEBUG-low
Failures wear WARN or ERROR glow
The rabbit hops, then watches flow
With tidy logs in rows below

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants