Merge pull request #630 from frankieramirez/feat/settings-log-viewer

feat: Read the logs from Settings, with an honest level dial

Author: frankieramirez

.changeset/settings-log-viewer.md

@@ -0,0 +1,9 @@
+---
+"comicarr": minor
+---
+
+Read your logs without leaving Comicarr. Settings has a new **Logs** section: the tail of `comicarr.log` in a console you can filter by severity and copy straight into a bug report, with the log level dial sitting right above it. Raise the level, reproduce the problem, and read what happened — without a shell, a `docker exec`, or a restart.
+
+The dial is honest about who is in charge. If a `--log-level` flag or `COMICARR_LOG_LEVEL` is setting the level, the page says so, names the level actually running and the one the next restart will bring back, and explains that the value you save here applies immediately but will not survive that restart until the pin is removed. When nothing overrides it, the page stays quiet and the dial simply is what runs.
+
+The header shows where the log file lives and how much history is kept (`10 MB × 5 files`) so you can see the ceiling before turning verbosity up. You can pull the last 200, 1,000, or 5,000 lines. Provider secrets are still redacted before any line leaves the server, and only the current log file is read — rotated files stay where they are.

CLAUDE.md

@@ -61,6 +61,7 @@ Conventional PR titles keep history readable, but they do not control releases.
 - **Do NOT call `useReactTable` directly** - `no-restricted-imports` allows it only in `frontend/src/components/data-table/useTableState.ts`. Call `useTableState`, which wraps it so `getRowId` is required and row identity can never fall back to TanStack's index default. Tables still awaiting migration are listed in an `overrides` allowlist in `eslint.config.js`; that list only ever shrinks — never add to it.
 - **Do NOT reintroduce `GLOBAL_MESSAGES`** - The pre-EventBus message bus is retired. Deleting the declaration cannot make its return fail (Python creates the attribute on first assignment), so `npm run lint:guards` scans source for it instead. Narrate through `comicarr.app.activity.events.record_activity`; that facade publishes the single `activity` SSE event after a durable commit. Add further retired names to `RETIRED_GLOBALS` in `scripts/check_retired_globals.py`. Contributor-only gate — no changeset.
 - **Log verbosity has exactly one dial** - `comicarr.LOG_LEVEL` (0/1/2) resolves through `logger.threshold_for_level()` and is applied identically to the logger, file, console, and Web UI sinks; ask for the current value with `logger.current_log_level()`, never by reading a global. Level 0 means warnings and errors, not silence. Whether a console sink exists at all is the orthogonal `console=` argument to `initLogger()`. The second dial (`comicarr.QUIET`) is retired and guarded by `RETIRED_GLOBALS`; it caused #610, where raising verbosity under Docker *removed* console output. Contract and history: `docs/architecture/logging-levels.md`.
+- **Do NOT show the saved log level as if it were the running one** - Settings writes `LOG_LEVEL` at the *bottom* of the precedence chain and the write applies live, so the running level, the saved level, and the level the next restart resolves to can all differ. Any surface that reports the level must use `resolve_effective_log_level` (which returns all three plus `pinned`) rather than reading `config.log_level` alone; `GET /system/logs` already carries it. Showing one number where three exist is #610 restated in the UI.
 - **`db.upsert` / `db.upsert_conn` table names must be lowercase `TABLE_MAP` keys** - The table is resolved by dict lookup, so `"Issues"` for `"issues"` lints clean and raises `ValueError: Unknown table for upsert` only when that write branch runs — it broke series refresh in production (#561). `scripts/check_upsert_tables.py` (under `lint:guards`) AST-scans every literal table argument. Runtime-built names are skipped; if you must build one, lowercase it at the source. Contributor-only gate — no changeset.
 - **Every `fail_reason` base token must be classified** in `comicarr/app/activity/reasons.py` before merge (`scripts/check_fail_reason_registry.py` under `lint:guards`). Runtime is fail-open; CI is the gate. Excluding a token requires a reconciliation obligation (never leave `Snatched`). See ADR-0001 / #523 / #541.
 - **Do NOT make a handoff route depend on the download client reaching back into Comicarr** - A handoff delivers the content, never a pointer to Comicarr, and must be verifiable from the client's own response alone (ADR-0002 / #552 / #564). `blackhole` and `watchdir` are the named exceptions to *verifiability* only — they pay for it by staying out of `_RESTART_SAFE_ROUTES`. There is no cheap static signal for "callback URL", so this is a review gate, not a lint one.

comicarr/app/config/log_level.py

@@ -41,6 +41,9 @@
 
 MIN_LEVEL = 0
 MAX_LEVEL = 2
+# What the dial sits at when nothing supplies a level. Matches the `LOG_LEVEL`
+# default in the config registry.
+DEFAULT_LEVEL = 1
 
 # One name per level, and the threshold picks it: level 0 is `WARNING`, so it is
 # called `warning`. The tempting `quiet`/`normal`/`verbose` triple is not here on
@@ -77,6 +80,30 @@ class LogLevelResolution:
     notices: list[str] = field(default_factory=list)
 
 
+@dataclass
+class EffectiveLogLevel:
+    """What the process is logging at now, and what a restart would make of it.
+
+    The Settings dial writes `config.ini`, the *bottom* of the chain, and the
+    write applies live. So three numbers can disagree at once: the level running
+    right now, the level saved in the config file, and the level the next start
+    resolves to. `pinned` is the one an operator needs -- when it is true, a
+    source the UI cannot edit wins the chain, and the dial's value will not
+    survive a restart. That is the #610 failure said out loud instead of
+    discovered later.
+    """
+
+    level: int
+    saved: int
+    restart_level: int
+    restart_source: str
+
+    @property
+    def pinned(self) -> bool:
+        """True when something above the config file decides the level."""
+        return self.restart_source in (SOURCE_ARGUMENT, SOURCE_ENVIRONMENT)
+
+
 def clamp_level(level: int) -> int:
     """Hold a level inside the dial's range, matching `threshold_for_level`."""
     return max(MIN_LEVEL, min(MAX_LEVEL, level))
@@ -130,6 +157,25 @@ def parse_level(raw, origin: str) -> tuple[int | None, list[str]]:
     return clamped, notices
 
 
+# The one thing about the chain that cannot be re-read later. The environment
+# and the config file are still there to be consulted at any moment; the startup
+# argument is consumed once at boot and `comicarr.LOG_LEVEL` is overwritten with
+# the resolved level straight after, so without this the Settings page could not
+# tell an operator that a `--log-level` flag is what keeps overruling their dial.
+_startup_argument: int | None = None
+
+
+def record_startup_argument(level) -> None:
+    """Remember the level a startup argument supplied, if one did."""
+    global _startup_argument
+    _startup_argument = level
+
+
+def startup_argument() -> int | None:
+    """The level the startup argument supplied, or None when none was passed."""
+    return _startup_argument
+
+
 def resolve_startup_log_level(
     argument_level=None,
     config_level=None,
@@ -153,7 +199,7 @@ def resolve_startup_log_level(
             supplied.append((level, source))
 
     if not supplied:
-        return LogLevelResolution(level=1, source=SOURCE_DEFAULT, notices=notices)
+        return LogLevelResolution(level=DEFAULT_LEVEL, source=SOURCE_DEFAULT, notices=notices)
 
     level, source = supplied[0]
     # Say what lost, so an operator who edits the Settings dial and sees nothing
@@ -166,3 +212,32 @@ def resolve_startup_log_level(
     if overridden:
         notices.append(f"Log level {describe_level(level)} from {source} overrides {', '.join(overridden)}.")
     return LogLevelResolution(level=level, source=source, notices=notices)
+
+
+def resolve_effective_log_level(
+    running_level,
+    config_level=None,
+    environ: Mapping[str, str] | None = None,
+) -> EffectiveLogLevel:
+    """Report the running level next to the one a restart would resolve to.
+
+    The restart half is the startup chain run again, now: the argument is the
+    recorded one, the environment and the config file are read fresh. Re-running
+    it rather than replaying the boot resolution is what keeps the answer true
+    after the operator saves a new level -- the chain's bottom rung has moved.
+    """
+    restart = resolve_startup_log_level(
+        argument_level=startup_argument(),
+        config_level=config_level,
+        environ=environ,
+    )
+    # A config file with no usable `LOG_LEVEL` still has a dial position: the
+    # registry default. Borrowing the resolved level instead would show the
+    # operator a startup argument's value in a field that edits the config.
+    saved, _ = parse_level(config_level, SOURCE_CONFIG)
+    return EffectiveLogLevel(
+        level=clamp_level(int(running_level or 0)),
+        saved=DEFAULT_LEVEL if saved is None else saved,
+        restart_level=restart.level,
+        restart_source=restart.source,
+    )

comicarr/app/config/registry.py

@@ -260,8 +260,12 @@ def as_definition(self) -> tuple[type, str, Any]:
     ConfigKey("MAL_ENABLED", bool, "MAL", False, readable=True, writable=True),
     ConfigKey("MAL_CLIENT_ID", str, "MAL", None, writable=True),
     ConfigKey("LOG_DIR", str, "Logs", None, readable=True),
-    ConfigKey("MAX_LOGSIZE", int, "Logs", 10000000),
-    ConfigKey("MAX_LOGFILES", int, "Logs", 5),
+    # Readable, never writable: Settings → Logs shows the retention ceiling as
+    # context because turning the level up to debug fills it fast, but the knob
+    # itself is one almost nobody should turn and every writable key is a
+    # permanent API surface.
+    ConfigKey("MAX_LOGSIZE", int, "Logs", 10000000, readable=True),
+    ConfigKey("MAX_LOGFILES", int, "Logs", 5, readable=True),
     ConfigKey("LOG_LEVEL", int, "Logs", 1, readable=True, writable=True),
     ConfigKey("GIT_PATH", str, "Git", None),
     ConfigKey("GIT_USER", str, "Git", "frankieramirez"),

comicarr/app/system/router.py

@@ -18,7 +18,7 @@
 import json
 import threading
 
-from fastapi import APIRouter, Depends, Request, Response
+from fastapi import APIRouter, Depends, Query, Request, Response
 from fastapi.responses import JSONResponse
 from sse_starlette.sse import EventSourceResponse, ServerSentEvent
 
@@ -346,9 +346,17 @@ def check_version_now(ctx: AppContext = Depends(get_context)):
 
 
 @router.get("/system/logs", dependencies=[Depends(require_session)])
-def get_logs(ctx: AppContext = Depends(get_context)):
-    """Return recent log entries."""
-    return system_service.get_recent_logs(ctx)
+def get_logs(
+    lines: int = Query(
+        system_service.DEFAULT_LOG_LINES,
+        ge=1,
+        le=system_service.MAX_LOG_LINES,
+        description="How many trailing lines of comicarr.log to return.",
+    ),
+    ctx: AppContext = Depends(get_context),
+):
+    """Return the tail of the current log file plus the effective log level."""
+    return system_service.get_recent_logs(ctx, lines=lines)
 
 
 @router.get("/system/jobs", dependencies=[Depends(require_session)])

comicarr/app/system/service.py

@@ -26,7 +26,7 @@
 import subprocess
 import sys
 import threading
-from collections import namedtuple
+from collections import deque, namedtuple
 from pathlib import Path
 from urllib.parse import urlsplit, urlunsplit
 
@@ -44,7 +44,13 @@
 from comicarr.app.acquisition.models import DispatchState
 from comicarr.app.common.dates import normalize_utc_datetime
 from comicarr.app.common.redaction import redact_sensitive_text
-from comicarr.app.config.log_level import ACCEPTED_FORMS, SOURCE_SETTINGS, parse_level
+from comicarr.app.config.log_level import (
+    ACCEPTED_FORMS,
+    NAME_FOR_LEVEL,
+    SOURCE_SETTINGS,
+    parse_level,
+    resolve_effective_log_level,
+)
 from comicarr.app.config.registry import (
     readable_keys,
     scheduler_job_intervals,
@@ -830,32 +836,77 @@ def get_build_identity(ctx):
     }
 
 
-def get_recent_logs(ctx):
-    """Return recent log entries."""
+# How many trailing lines Settings → Logs asks for by default, and the ceiling
+# on what it may ask for. The file is capped at MAX_LOGSIZE anyway; the ceiling
+# is here so one request cannot be made to hold an entire rotation in memory.
+DEFAULT_LOG_LINES = 200
+MAX_LOG_LINES = 5000
+
+
+def _log_level_context(ctx):
+    """The three levels the Settings dial has to be honest about.
+
+    `saved` is what the dial edits, `effective` is what the process is logging
+    at this second, and `restart` is what the startup chain resolves to next
+    time. They can all differ, and #610 is what happens when the UI shows only
+    the first one.
+    """
+    config_level = getattr(ctx.config, "LOG_LEVEL", None) if ctx.config else None
+    effective = resolve_effective_log_level(logger.current_log_level(), config_level=config_level)
+    return {
+        "effective": effective.level,
+        "effective_name": NAME_FOR_LEVEL[effective.level],
+        "saved": effective.saved,
+        "saved_name": NAME_FOR_LEVEL[effective.saved],
+        "restart_level": effective.restart_level,
+        "restart_name": NAME_FOR_LEVEL[effective.restart_level],
+        "restart_source": effective.restart_source,
+        "pinned": effective.pinned,
+    }
+
+
+def get_recent_logs(ctx, lines=DEFAULT_LOG_LINES):
+    """Return the tail of `comicarr.log`, with the level context the dial needs.
+
+    Only the current file: rotated `comicarr.log.1` and friends are deliberately
+    unreachable here, and there is no pagination — the surface exists so an
+    operator can raise the level, reproduce, and paste, not to browse history.
+    """
+    requested = max(1, min(int(lines or DEFAULT_LOG_LINES), MAX_LOG_LINES))
+    level = _log_level_context(ctx)
+
     log_dir = getattr(ctx.config, "LOG_DIR", None) if ctx.config else None
     if not log_dir:
         log_dir = os.path.join(ctx.data_dir, "logs") if ctx.data_dir else None
 
     if not log_dir:
-        return {"logs": []}
+        return {"logs": [], "level": level, "requested": requested, "path": None}
 
     log_file = os.path.join(log_dir, "comicarr.log")
     if not os.path.exists(log_file):
-        return {"logs": []}
+        return {"logs": [], "level": level, "requested": requested, "path": log_file}
 
     try:
+        # A deque with a maxlen keeps only the tail in memory. `readlines()` on a
+        # 10 MB log allocated the whole file on every Refresh, and the viewer was
+        # always going to throw all but the last N away.
         with open(log_file, "r") as f:
-            lines = f.readlines()
+            tail = deque(f, maxlen=requested)
         provider_secrets = []
         if ctx.config:
             for attr_name in ("EXTRA_NEWZNABS", "EXTRA_TORZNABS"):
                 for entry in getattr(ctx.config, attr_name, []) or []:
                     if isinstance(entry, (list, tuple)) and len(entry) > 3:
                         provider_secrets.append(entry[3])
-        return {"logs": [redact_sensitive_text(line, provider_secrets) for line in lines[-200:]]}
+        return {
+            "logs": [redact_sensitive_text(line, provider_secrets) for line in tail],
+            "level": level,
+            "requested": requested,
+            "path": log_file,
+        }
     except Exception as e:
         logger.error("[SYSTEM] Error reading logs: %s" % e)
-        return {"logs": [], "error": str(e)}
+        return {"logs": [], "level": level, "requested": requested, "path": log_file, "error": str(e)}
 
 
 def get_job_info(ctx, include_acquisition=True):

comicarr/config.py

@@ -35,7 +35,7 @@
 
 import comicarr
 from comicarr import db, encrypted, filechecker, helpers, logger, maintenance
-from comicarr.app.config.log_level import resolve_startup_log_level
+from comicarr.app.config.log_level import record_startup_argument, resolve_startup_log_level
 from comicarr.app.config.registry import as_legacy_definitions
 
 config = configparser.ConfigParser()
@@ -415,6 +415,10 @@ def read(self, startup=False):
             # COMICARR_LOG_LEVEL, then the config file. See
             # comicarr/app/config/log_level.py and
             # docs/architecture/logging-levels.md.
+            # Capture the argument before the line below overwrites the global
+            # with the resolved level: the Settings page has to be able to name
+            # a `--log-level` flag as the reason its dial keeps losing.
+            record_startup_argument(comicarr.LOG_LEVEL)
             resolution = resolve_startup_log_level(
                 argument_level=comicarr.LOG_LEVEL,
                 config_level=self.LOG_LEVEL,

docs/architecture/logging-levels.md

@@ -125,6 +125,30 @@ consequence is what stops the next "I set it to 0 and expected silence". This
 supersedes the `quiet / normal / verbose` labels locked while designing the
 surface; the layout of that design is unaffected.
 
+### The dial must say when it is not the one deciding
+
+Because Settings writes the bottom rung of the chain and the write applies live,
+three numbers can disagree at once: the level the process is logging at, the
+level saved in `config.ini`, and the level the next start will resolve to. A
+page that shows only the saved number is #610 in miniature — the UI stating one
+thing while the process does another.
+
+`resolve_effective_log_level` (`comicarr/app/config/log_level.py`) reports all
+three, and `GET /system/logs` carries them alongside the log lines. The restart
+half is the startup chain *re-run now*, not a replay of the boot resolution:
+the environment and the config file are read fresh, and only the startup
+argument is remembered, because it is the one input that cannot be re-read
+later. `record_startup_argument` captures it in `comicarr/config.py` before
+`comicarr.LOG_LEVEL` is overwritten with the resolved level.
+
+`pinned` is true when the winner is a startup argument or `COMICARR_LOG_LEVEL`.
+Settings → Logs shows its override callout on exactly that flag, and on nothing
+else: when the config file is the top of the chain there is nothing to say, and
+an always-visible status card would be noise. Note that `pinned` is a statement
+about *source*, not about a mismatch in numbers — `--log-level info` against a
+saved `1` shows no disagreement and still means the dial cannot change what a
+restart does.
+
 ### `--quiet` and `--verbose`
 
 Both are deprecated aliases — `--quiet` for `--log-level warning`, `--verbose`