Replace print() error output with structured logging (#68) by bradjin8 · Pull Request #77 · cppalliance/cppa-cursor-browser · GitHub
Skip to content

Replace print() error output with structured logging (#68)#77

Merged
bradjin8 merged 9 commits into
masterfrom
feat/replace-print-with-logging
May 26, 2026
Merged

Replace print() error output with structured logging (#68)#77
bradjin8 merged 9 commits into
masterfrom
feat/replace-print-with-logging

Conversation

@bradjin8

@bradjin8 bradjin8 commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Configure default application logging in create_app() (logging.basicConfig at INFO, format includes module and function name for gunicorn/WSGI capture).
  • Replace all error-reporting print() calls in services/ and api/ with module loggers (logging.getLogger(__name__)) using warning for schema drift and error with exc_info=True for unexpected failures.
  • Remove traceback.print_exc() from export/PDF handlers in favor of structured traceback via exc_info=True.
  • Update test_bubble_schema_drift_is_logged_not_swallowed_silently to assert on log output (assertLogs) instead of stdout.

Closes #68. Complements #66 (except-pass → logging): services that already logged parse failures are unchanged; this PR finishes Test 10 coverage for paths that still used print().

Files touched: app.py, services/cli_tabs.py, api/composers.py, api/search.py, api/export_api.py, api/pdf.py, api/config_api.py, tests/test_models_wired_at_read_sites.py

Out of scope: scripts/export.py CLI stderr warnings (not under services/ / api/ per issue).

Test plan

  • python -m pytest -q — 306 passed, 4 skipped
  • No print( calls remain in services/ or api/
  • Manual: start app locally, trigger a schema-drift row (or bad CLI session path) and confirm WARNING/ERROR lines appear in server logs with module name and entity id
  • Reviewer: confirm log format is acceptable behind gunicorn per DEPLOYMENT.md

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling and structured logging so parse/schema failures and other internal errors emit contextual warnings/errors and problematic items are skipped without breaking user flows.
  • Chores

    • Updated Click dependency to 8.4.1.
    • Initialized application logging configuration for consistent log output.
  • Tests

    • Added and updated tests to verify resilient behavior and that warnings are emitted for parse/schema/format issues.

Review Change Stack

@bradjin8 bradjin8 self-assigned this May 25, 2026
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🤖 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 `@services/workspace_listing.py`:
- Line 137: The top-level composer load error log currently drops stack context
by calling _logger.error("Failed to load composer rows from global storage: %s",
e); update that call to include the exception traceback by passing exc_info=True
(i.e., _logger.error(..., exc_info=True)) so the stack trace is preserved for
operational debugging while keeping the existing message and exception
interpolation.

In `@services/workspace_tabs.py`:
- Around line 126-131: The warning currently logs the raw cursorDiskKV payload
(row["value"]) which can leak sensitive conversation data and be very large;
update the _logger.warning call(s) that reference row["value"] (the one shown
and the similar block around lines 210-215) to avoid including the raw payload
and instead log stable identifiers and payload size — keep row["key"] (or any
available stable id like bubble id) and add payload length (e.g.,
len(row.get("value", b"")) or length of the string) and/or a checksum/short hash
if you need a stable fingerprint; update the _logger.warning invocations in
workspace_tabs.py that currently format row["value"] accordingly.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: aced6938-416d-42b6-b9e3-5c4e0857d0f0

📥 Commits

Reviewing files that changed from the base of the PR and between 731c1e1 and 2a7c44e.

📒 Files selected for processing (17)
  • api/composers.py
  • api/config_api.py
  • api/export_api.py
  • api/logs.py
  • api/pdf.py
  • api/search.py
  • api/workspaces.py
  • app.py
  • requirements-lock.txt
  • services/cli_tabs.py
  • services/workspace_listing.py
  • services/workspace_resolver.py
  • services/workspace_tabs.py
  • tests/test_invalid_workspace_aliases.py
  • tests/test_models_wired_at_read_sites.py
  • tests/test_parse_failure_logging.py
  • tests/test_workspace_tabs_null_bubble.py

Comment thread services/workspace_listing.py Outdated
Comment thread services/workspace_tabs.py

@coderabbitai coderabbitai Bot 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.

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 `@services/workspace_tabs.py`:
- Around line 138-145: The JSON decode guard around json.loads in
services/workspace_tabs.py currently only catches json.JSONDecodeError; change
those except clauses (the one around the block that calls _kv_payload_log_meta
and logs via _logger.warning with row["key"] and e) to catch
(json.JSONDecodeError, TypeError, ValueError) so non-text SQLite values or other
decode issues are treated non-fatal; apply the same change to the other
symmetric decode block that logs payload_len/payload_fp (the second handler that
currently only catches JSONDecodeError) so both malformed KV row paths behave
consistently.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65d3d309-c7f4-4f28-8a15-8bc988cd7bf5

📥 Commits

Reviewing files that changed from the base of the PR and between 2a7c44e and 048686a.

📒 Files selected for processing (2)
  • services/workspace_listing.py
  • services/workspace_tabs.py

Comment thread services/workspace_tabs.py
@bradjin8 bradjin8 force-pushed the feat/replace-print-with-logging branch from 7238424 to e97f28e Compare May 25, 2026 19:53
@bradjin8 bradjin8 requested a review from clean6378-max-it May 25, 2026 20:14
@clean6378-max-it

Copy link
Copy Markdown
Collaborator

@clean6378-max-it clean6378-max-it requested a review from wpak-ai May 25, 2026 21:13
@bradjin8 bradjin8 force-pushed the feat/replace-print-with-logging branch from 33d0478 to 468aacc Compare May 26, 2026 15:47
@bradjin8 bradjin8 merged commit e6bae8c into master May 26, 2026
16 checks passed
@bradjin8 bradjin8 deleted the feat/replace-print-with-logging branch May 26, 2026 15:48
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.

Replace print() error output with structured logging

3 participants