Skip to content

🐛 Route analyse/ logging by environment instead of a stderr handler at import#73

Merged
ubmarco merged 6 commits into
mainfrom
fix/72-analyse-logging-env-routing
Jun 8, 2026
Merged

🐛 Route analyse/ logging by environment instead of a stderr handler at import#73
ubmarco merged 6 commits into
mainfrom
fix/72-analyse-logging-env-routing

Conversation

@ubmarco

@ubmarco ubmarco commented Jun 7, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #72. The analyse/ layer installed a logging.StreamHandler on its own loggers at import time, so routine INFO progress (Source files loaded: N, …) went to stderr unconditionally — reddened by tox/CI even though the build succeeds (exit 0) — and importing the package took the handler/level/stream choice away from consumers (against the stdlib guidance for libraries).

This adds an environment-aware logging facade and routes the shared analyse/ layer through whichever frontend is active, modelled on sphinx_needs/logging.py.

What changed

  • logger.py — new get_logger(name) facade over a swappable backend:
    • default (plain library): stdlib logging, no handler installed → INFO dropped silently, WARNING+ to stderr via logging.lastResort.
    • CLI: the existing rich logger → INFO to stdout, WARNING to stderr, honouring --verbose/--quiet.
    • Sphinx: sphinx.util.logging → progress at VERBOSE, warnings carrying type="codelinks" + a subtype (suppressible via suppress_warnings, on the Sphinx warning stream).
  • analyse/{analyse,utils,projects,oneline_parser}.py — drop the import-time handler; use get_logger(__name__). utils.py's git warnings (previously logging.warning() on the root logger, which triggers basicConfig) now go through the facade with subtypes. oneline_parser.py's dead logging block (no log calls) is removed.
  • Entry points — the CLI analyse command gains -v/--verbose & -q/--quiet and calls configure_cli(...); the Sphinx setup() calls configure_sphinx().

Behaviour

Environment INFO progress WARNING
plain library (no configure) dropped stderr (lastResort)
CLI (default) stdout stderr
CLI --quiet hidden stderr
Sphinx (normal build) hidden Sphinx warning stream (codelinks.<subtype>)
Sphinx -v Sphinx status stream Sphinx warning stream

The accessible-pygments double-print noted in the issue disappears too, since codelinks no longer emits INFO that propagates to the root logger. propagate is left untouched (so consumers can still capture our logs).

Testing

  • New tests/test_logger.py: no handler installed at import (regression guard for analyse/* installs a stderr logging handler at import; routine INFO progress pollutes stderr (renders red under tox/CI) #72); default mode drops INFO / emits WARNING; CLI routes INFO→stdout & WARNING→stderr with --quiet suppressing INFO; Sphinx routes via SphinxLoggerAdapter with type/subtype.
  • New CLI test in tests/test_cmd.py for the analyse progress / --quiet gating.
  • Full suite green (212 passed), mypy --strict clean, ruff clean on changed files.
  • Manual repro — real sphinx-build of tests/data/sphinx: the INFO progress no longer appears on stderr (was flooded before, including the doubled accessible-pygments lines); only genuine warnings remain.

ubmarco added 3 commits June 7, 2026 23:50
…t import

The analyse/ modules installed a logging.StreamHandler on their own loggers at
import time, sending routine INFO progress (`Source files loaded: N`, ...) to
stderr unconditionally. Wrappers that colour stderr (tox, CI log viewers)
rendered these harmless lines red, and importing the package took the
handler/level/stream choice away from consumers — against the stdlib logging
guidance for libraries.

Add an environment-aware logging facade in logger.py (`get_logger` + a
swappable backend) and route the analyse/ layer through it:

- default (plain library): stdlib logging with no handler installed, so INFO is
  dropped and WARNING+ reaches stderr via logging.lastResort.
- CLI: the existing rich logger — INFO to stdout, WARNING to stderr, honouring
  --verbose/--quiet (now added to the `analyse` command too).
- Sphinx: sphinx.util.logging — progress at VERBOSE, warnings carrying
  type="codelinks" + a subtype so they are suppressible and rendered on the
  Sphinx warning stream (mirrors sphinx-needs' logging helper).

utils.py's git warnings previously used logging.warning() on the root logger
(triggering basicConfig); they now go through the facade with subtypes. The
dead logging block in oneline_parser.py (no log calls) is removed.

Closes #72
…Runner mix_stderr)

typer >= 0.24 (CI resolved 0.26.7) removed `mix_stderr` from `CliRunner`, so
error output from `typer.BadParameter` now lands only on stderr and
`result.stdout` is empty for error cases. The negative CLI tests asserted on
`result.stdout`, so they began failing on every matrix cell once CI picked up
the newer typer (independent of the logging change — they fail on main too).

`result.output` carries the error text under both old and new typer, so use it
for the error-path assertions. Success-output assertions (test_discover) are
unaffected and left as-is.
…g test need link

Routing the analyse layer's warnings through Sphinx (this PR) means they now
reach the docs build, which runs `sphinx-build -nW` (warnings as errors). Two
problems surfaced:

- In a detached HEAD (CI checkouts), `.git/HEAD` holds the commit SHA directly.
  `get_current_rev` treated that as an error, warned (`codelinks.git_head`) and
  returned None — tripping `-W`. The SHA *is* the rev, so return it. This also
  yields correct remote source URLs in detached-HEAD builds. Adds a regression
  test for the detached-HEAD case.
- The codelinks comment atop tests/test_logger.py declared need TEST_LOG_1 with
  a link to a non-existent IMPL_LOG_ROUTING, producing a sphinx-needs
  unknown-link warning. Drop the traceability comment (no matching impl need
  exists to link to).
@codecov-commenter

codecov-commenter commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.39175% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.79%. Comparing base (a65483e) to head (74551fc).

Files with missing lines Patch % Lines
src/sphinx_codelinks/analyse/utils.py 44.44% 5 Missing ⚠️
src/sphinx_codelinks/logger.py 96.22% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   90.06%   90.79%   +0.72%     
==========================================
  Files          29       30       +1     
  Lines        2628     2771     +143     
  Branches      306      305       -1     
==========================================
+ Hits         2367     2516     +149     
+ Misses        165      160       -5     
+ Partials       96       95       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ubmarco added 3 commits June 8, 2026 08:11
…down behind -v

By default a Sphinx build showed nothing from codelinks (progress was emitted at
VERBOSE, i.e. only with `sphinx-build -v`), and even then the lines lacked
project context.

Emit a per-project marker from `SourceAnalyse.run()` — used by both the CLI
(`AnalyseProjects`) and the Sphinx `src-trace` directive — so it appears in both
frontends:

- INFO (default-visible): `codelinks [<project>]: <N> files, <M> markers`
  (Sphinx status stream / CLI stdout, silenced by `--quiet`).
- DEBUG (only with `-v` / `--verbose`): per-type breakdown — comments,
  oneline-needs, id-refs, marked-rst.

`SourceAnalyse` gains a `name` kwarg (the project) which both callers pass. Adds
a `debug` level to the logging facade (Sphinx `verbose()`, CLI rich `debug()`),
and switches the Sphinx `info` mapping from `verbose()` to `info()` so the
summary is visible in a normal build. The context-less per-step logging in
`analyse.py` is removed in favour of the labelled summary.
"1 file, 1 marker" instead of "1 files, 1 markers"; applies to the default
summary and the -v breakdown (comments / oneline needs / id-refs / marked-rst
blocks) via a small `_count` helper.
The pluralization test imported the helper inside the function; PLC0415 is only
ignored for cmd.py, so the in-function import failed the ruff pre-commit hook.
Move it to the top-level import.
@ubmarco ubmarco requested a review from patdhlk June 8, 2026 11:18
@ubmarco ubmarco merged commit c9e78da into main Jun 8, 2026
13 checks passed
@ubmarco ubmarco deleted the fix/72-analyse-logging-env-routing branch June 8, 2026 12:16
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.

analyse/* installs a stderr logging handler at import; routine INFO progress pollutes stderr (renders red under tox/CI)

3 participants