feat: add TRACE log level and patch_logging()#20
Conversation
Port TRACE log level support from appier (hivesolutions/appier#84) to colony. Adds TRACE constant (logging.DEBUG - 5 = 5) and SILENT constant (logging.CRITICAL + 1 = 51) for fine-grained protocol-level debugging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds TRACE and SILENT logging levels and a patching utility that registers TRACE and Logger.trace, exposes these at package level, integrates trace-aware formatting and trace methods into PluginManager, adds tests, a package main entry, and renames an internal helper in colony_adm. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant PM as PluginManager
participant Loggers as colony.base.loggers
participant Python as Python logging
App->>PM: start_logger(log_level)
PM->>Loggers: patch_logging()
Loggers->>Python: addLevelName(TRACE, "TRACE")
Loggers->>Python: set Logger.trace = _trace
PM->>PM: configure handlers/formatters (use TRACE format if applicable)
App->>PM: trace("message", *args)
PM->>Python: logger.isEnabledFor(TRACE)
alt TRACE enabled
Python-->>PM: True
PM->>Python: logger.log(TRACE, message, *args, stacklevel=2)
Python->>Python: Emit record (with pathname/lineno)
else TRACE disabled
Python-->>PM: False
PM-->>App: return (no emit)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Adds trace() convenience method to PluginManager and makes all log methods (debug, info, warning, error, critical) accept *args and **kwargs for lazy evaluation support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e64ed5430e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| or an handler, this is used as an utility for debugging | ||
| purposes more that a real feature for production systems """ | ||
|
|
||
| TRACE = logging.DEBUG - 5 |
There was a problem hiding this comment.
Adding TRACE introduces a new record level, but MemoryHandler.get_messages_l() still indexes into LEVELS (DEBUG..CRITICAL) and returns an empty deque for unknown levels. As a result, TRACE records are never inserted into messages_l, so get_latest(level="TRACE") (or level 5 after patching) always returns no entries even when TRACE logs were emitted. This breaks level-filtered log retrieval for the new level.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds support for a TRACE logging level (and SILENT helper level) across Colony’s logging utilities, integrating it into Python’s logging via a new patch_logging() hook and exposing trace helpers in the plugin manager API.
Changes:
- Introduces
TRACE/SILENTlevel constants and updates level name/int resolution soLEVEL=TRACEworks. - Adds
patch_logging()to register TRACE with Pythonloggingand installs aLogger.trace()method (called fromstart_logger()). - Extends logging APIs/tests (new
PluginManager.is_trace(),DummyLogger.trace(), and new/updated test coverage).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/colony/test/base/loggers.py | Adds tests for TRACE/SILENT constants, patch_logging(), and trace logging behavior |
| src/colony/libs/logging_util.pyi | Updates typing surface to include TRACE/SILENT and DummyLogger.trace() |
| src/colony/libs/logging_util.py | Adds TRACE/SILENT constants and level mapping support |
| src/colony/libs/init.py | Re-exports TRACE/SILENT from libs |
| src/colony/base/system.pyi | Expands typed logging API to include trace and passthrough args/kwargs; adds is_trace() |
| src/colony/base/system.py | Calls patch_logging() from start_logger(), adds PluginManager.trace() and is_trace() |
| src/colony/base/loggers.py | Defines TRACE/SILENT constants and implements patch_logging() + Logger.trace() |
| src/colony/base/init.py | Re-exports TRACE/SILENT and patch_logging() from base |
| CHANGELOG.md | Documents the addition of TRACE log level and patch_logging() |
Comments suppressed due to low confidence (1)
src/colony/base/loggers.py:81
MemoryHandler.get_messages_l()relies onLEVELSto recognize record.levelname values. With TRACE introduced, a TRACE record will not be added to any per-level deque (andget_latest(level="TRACE")will always return empty) becauseLEVELSdoes not include "TRACE". Consider extendingLEVELS(and any related aliasing) to include TRACE so in-memory filtering works for the new level.
TRACE = logging.DEBUG - 5
""" The trace level used for extremely detailed and verbose
logging of protocol-level operations, this is meant to be
used for fine-grained debugging of low-level operations
like raw byte transfers and frame parsing """
LEVELS = ("DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL")
""" The sequence of levels from the least sever to the
most sever this sequence may be used to find all the
levels that are considered more sever that a level """
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not self.logger.level: | ||
| return False | ||
| return self.logger.level <= loggers.TRACE |
There was a problem hiding this comment.
is_trace() currently returns False when self.logger.level is 0 (NOTSET), even though the logger may still effectively be enabled for TRACE via inheritance from parent/root. Using self.logger.getEffectiveLevel() (or self.logger.isEnabledFor(loggers.TRACE)) would make this check accurate in common NOTSET configurations.
| if not self.logger.level: | |
| return False | |
| return self.logger.level <= loggers.TRACE | |
| return self.logger.isEnabledFor(loggers.TRACE) |
| def get_author_name(self): ... | ||
| def get_uptime(self): ... | ||
| def log_stack_trace(self, level=...) -> None: ... | ||
| def debug(self, message) -> None: ... | ||
| def info(self, message) -> None: ... | ||
| def warning(self, message) -> None: ... | ||
| def error(self, message) -> None: ... | ||
| def critical(self, message) -> None: ... | ||
| def trace(self, message, *args, **kwargs) -> None: ... | ||
| def debug(self, message, *args, **kwargs) -> None: ... | ||
| def info(self, message, *args, **kwargs) -> None: ... | ||
| def warning(self, message, *args, **kwargs) -> None: ... | ||
| def error(self, message, *args, **kwargs) -> None: ... | ||
| def critical(self, message, *args, **kwargs) -> None: ... |
There was a problem hiding this comment.
system.pyi declares Plugin.trace(...), but the runtime Plugin implementation in system.py does not define a corresponding trace() method (only PluginManager.trace() exists). Either add trace() to Plugin for parity with the stub/public API, or remove it from the Plugin stub to avoid type/runtime mismatch.
| """ The silent level used to silent all the logging | ||
| or an handler, this is used as an utility for debugging | ||
| purposes more that a real feature for production systems """ |
There was a problem hiding this comment.
Docstring grammar typos in the newly added SILENT description (e.g., "silent all the logging", "more that"). Consider correcting to "silence all logging" / "more than" to keep docs polished and searchable.
| """ The silent level used to silent all the logging | |
| or an handler, this is used as an utility for debugging | |
| purposes more that a real feature for production systems """ | |
| """ The silent level used to silence all logging | |
| or a handler, this is used as a utility for debugging | |
| purposes more than a real feature for production systems """ |
| """ The silent level used to silent all the logging | ||
| or an handler, this is used as an utility for debugging | ||
| purposes more that a real feature for production systems """ |
There was a problem hiding this comment.
Docstring grammar typos in the newly added SILENT description (e.g., "silent all the logging", "more that"). Consider correcting to "silence all logging" / "more than".
| """ The silent level used to silent all the logging | |
| or an handler, this is used as an utility for debugging | |
| purposes more that a real feature for production systems """ | |
| """ The silent level used to silence all logging | |
| or a handler, this is used as a utility for debugging | |
| purposes more than a real feature for production systems """ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/colony/base/system.pyi (1)
44-53:⚠️ Potential issue | 🔴 CriticalAdd
trace()method toSystemclass stub.The
Systemclass in the stub file is missing thetrace()method signature that exists in the implementation. Add:def trace(self, message, *args, **kwargs) -> None: ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/colony/base/system.pyi` around lines 44 - 53, The System stub is missing the trace method signature present in the implementation; add a declaration for def trace(self, message, *args, **kwargs) -> None: ... to the System class in src/colony/base/system.pyi so the stub matches the runtime API (look for the System class and add the trace method alongside debug/info/warning/error/critical).src/colony/base/system.py (1)
6066-6085:⚠️ Potential issue | 🟡 MinorDon't double-log the traceback when
exc_infois provided.Now that these helpers forward
**kwargs, callers can passexc_info=True. The stdlib logger will already emit the exception traceback, and the unconditionallog_stack_trace()call appends the same frames again.🔧 Guard the log_stack_trace() call; apply to all three methods: warning(), error(), critical()
logger_message = self.format_logger_message(message) self.logger.error(logger_message, *args, **kwargs) - - # logs the stack trace - self.log_stack_trace(level=logging.WARNING) + if not kwargs.get("exc_info", False): + self.log_stack_trace(level=logging.WARNING)Also applies to:
warning()(6066-6085,level=logging.INFO) andcritical()(6108-6123,level=logging.ERROR).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/colony/base/system.py` around lines 6066 - 6085, The unconditioned call to log_stack_trace duplicates the traceback when callers pass exc_info=True; update warning(), error(), and critical() to check kwargs.get('exc_info') (or 'exc_info' in kwargs and truthy) and only call self.log_stack_trace(...) when exc_info is not provided/false; keep the existing log level arguments (warning uses level=logging.INFO, error/critical use level=logging.ERROR) and continue forwarding *args/**kwargs to self.logger so behavior is unchanged when exc_info is omitted.
🧹 Nitpick comments (2)
CHANGELOG.md (1)
12-12: Consider mentioningSILENTconstant in the changelog entry.The PR also adds a
SILENTlogging level constant, but onlyTRACEandpatch_logging()are documented here. For completeness, consider updating to include both new constants.📝 Suggested update
-* TRACE log level and `patch_logging()` for fine-grained protocol-level debugging +* TRACE and SILENT log level constants and `patch_logging()` for fine-grained protocol-level debugging🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 12, Update the changelog entry to mention the newly added SILENT logging level alongside TRACE and patch_logging(): edit the line containing "TRACE log level and `patch_logging()` for fine-grained protocol-level debugging" to include `SILENT` (e.g., "TRACE and SILENT log levels and `patch_logging()`...") so both new constants are documented; ensure you reference the SILENT constant name exactly and keep the style consistent with the existing changelog entries.src/colony/libs/logging_util.py (1)
31-55: Duplicated level constants may drift fromloggers.py.
SILENT = 51andTRACE = 5are hardcoded here, whilesrc/colony/base/loggers.pydefines them aslogging.CRITICAL + 1andlogging.DEBUG - 5respectively. While the values currently match, this duplication creates a maintenance risk if the base values ever change.Consider importing from
loggers.pyor using a shared definition to ensure consistency:♻️ Possible approaches
Option 1: Import from loggers (if no circular import issues):
from colony.base.loggers import SILENT, TRACEOption 2: Define using logging module constants here too:
+import logging + -SILENT = 51 +SILENT = logging.CRITICAL + 1 """ The silent level... """ -TRACE = 5 +TRACE = logging.DEBUG - 5 """ The trace level... """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/colony/libs/logging_util.py` around lines 31 - 55, The file defines duplicated constants SILENT and TRACE (SILENT = 51, TRACE = 5) which can drift from the canonical definitions; replace the hardcoded values by importing the shared definitions (e.g. from colony.base.loggers import SILENT, TRACE) or compute them from the logging module (SILENT = logging.CRITICAL + 1, TRACE = logging.DEBUG - 5) so the constants in logging_util.py (SILENT, TRACE) are always consistent with the base loggers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/colony/base/system.py`:
- Around line 6012-6029: The trace method currently calls
self.logger.log(loggers.TRACE, ...) which breaks when using DummyLogger (it
implements trace() but not log()); update the trace() implementation in the
trace method to call self.logger.trace(logger_message, *args, **kwargs) after
building logger_message with self.format_logger_message(message) so it matches
the other level methods (debug/info/warning/error/critical) and maintains
DummyLogger compatibility.
- Around line 6596-6611: The is_trace() method incorrectly inspects
self.logger.level directly (which may be 0/NOTSET for inherited levels); change
it to use the logger API so it respects effective-level semantics: verify
self.logger is not None and then return self.logger.isEnabledFor(loggers.TRACE)
(or False if no logger) so trace checks honor parent/inherited levels; update
references to is_trace, self.logger and loggers.TRACE accordingly.
---
Outside diff comments:
In `@src/colony/base/system.py`:
- Around line 6066-6085: The unconditioned call to log_stack_trace duplicates
the traceback when callers pass exc_info=True; update warning(), error(), and
critical() to check kwargs.get('exc_info') (or 'exc_info' in kwargs and truthy)
and only call self.log_stack_trace(...) when exc_info is not provided/false;
keep the existing log level arguments (warning uses level=logging.INFO,
error/critical use level=logging.ERROR) and continue forwarding *args/**kwargs
to self.logger so behavior is unchanged when exc_info is omitted.
In `@src/colony/base/system.pyi`:
- Around line 44-53: The System stub is missing the trace method signature
present in the implementation; add a declaration for def trace(self, message,
*args, **kwargs) -> None: ... to the System class in src/colony/base/system.pyi
so the stub matches the runtime API (look for the System class and add the trace
method alongside debug/info/warning/error/critical).
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 12: Update the changelog entry to mention the newly added SILENT logging
level alongside TRACE and patch_logging(): edit the line containing "TRACE log
level and `patch_logging()` for fine-grained protocol-level debugging" to
include `SILENT` (e.g., "TRACE and SILENT log levels and `patch_logging()`...")
so both new constants are documented; ensure you reference the SILENT constant
name exactly and keep the style consistent with the existing changelog entries.
In `@src/colony/libs/logging_util.py`:
- Around line 31-55: The file defines duplicated constants SILENT and TRACE
(SILENT = 51, TRACE = 5) which can drift from the canonical definitions; replace
the hardcoded values by importing the shared definitions (e.g. from
colony.base.loggers import SILENT, TRACE) or compute them from the logging
module (SILENT = logging.CRITICAL + 1, TRACE = logging.DEBUG - 5) so the
constants in logging_util.py (SILENT, TRACE) are always consistent with the base
loggers.
🪄 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: 6f5ce215-e001-46a1-9671-ef155054374a
📒 Files selected for processing (9)
CHANGELOG.mdsrc/colony/base/__init__.pysrc/colony/base/loggers.pysrc/colony/base/system.pysrc/colony/base/system.pyisrc/colony/libs/__init__.pysrc/colony/libs/logging_util.pysrc/colony/libs/logging_util.pyisrc/colony/test/base/loggers.py
| def is_trace(self): | ||
| """ | ||
| Checks if the current logging level is set to trace, | ||
| this check may be used to action conditional code | ||
| execution for fine-grained debugging purposes. | ||
|
|
||
| :rtype: bool | ||
| :return: Value indicating if the current logging level | ||
| is set to trace (for fine-grained debugging). | ||
| """ | ||
|
|
||
| if not self.logger: | ||
| return False | ||
| if not self.logger.level: | ||
| return False | ||
| return self.logger.level <= loggers.TRACE |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import logging
TRACE = logging.DEBUG - 5
parent = logging.getLogger("trace_parent")
child = logging.getLogger("trace_parent.child")
parent.setLevel(TRACE)
child.setLevel(logging.NOTSET)
raw = False if not child.level else child.level <= TRACE
print("raw check:", raw)
print("isEnabledFor:", child.isEnabledFor(TRACE))
child.disabled = True
raw = False if not child.level else child.level <= TRACE
print("raw check after disable:", raw)
print("isEnabledFor after disable:", child.isEnabledFor(TRACE))
PYRepository: hivesolutions/colony
Length of output: 164
🏁 Script executed:
# First, verify the file exists and check the specified lines
if [ -f "src/colony/base/system.py" ]; then
echo "=== File exists, checking lines 6596-6611 ==="
sed -n '6596,6611p' src/colony/base/system.py
else
echo "File not found at src/colony/base/system.py"
git ls-files | grep -i system.py
fiRepository: hivesolutions/colony
Length of output: 648
🏁 Script executed:
# Search for loggers.TRACE definition
echo "=== Searching for TRACE constant definition ==="
rg "TRACE\s*=" --type py -B 2 -A 2 | head -40Repository: hivesolutions/colony
Length of output: 781
🏁 Script executed:
# Check for loggers module and TRACE definition
echo "=== Looking for loggers module ==="
git ls-files | grep -i loggerRepository: hivesolutions/colony
Length of output: 157
🏁 Script executed:
# Check the class context and surrounding code
echo "=== Class context (lines 6580-6620) ==="
sed -n '6580,6620p' src/colony/base/system.pyRepository: hivesolutions/colony
Length of output: 1552
🏁 Script executed:
# Check if there are any logger initializations or type hints
echo "=== Searching for logger attribute initialization ==="
rg "self\.logger\s*=" src/colony/base/system.py -B 2 -A 2 | head -50Repository: hivesolutions/colony
Length of output: 512
🏁 Script executed:
# Verify the file uses CRLF or LF line endings
echo "=== Checking line endings ==="
file src/colony/base/system.pyRepository: hivesolutions/colony
Length of output: 155
🏁 Script executed:
# Check line endings more reliably
echo "=== Checking line endings (first 10 lines) ==="
head -10 src/colony/base/system.py | od -c | head -20Repository: hivesolutions/colony
Length of output: 168
🏁 Script executed:
# Find the class containing is_trace method
echo "=== Finding class definition ==="
grep -n "^class " src/colony/base/system.py | head -5
awk '/^class /{class=$0; line=NR} /def is_trace/{print "Line", line": "class}' src/colony/base/system.pyRepository: hivesolutions/colony
Length of output: 298
🏁 Script executed:
# Check for type annotations in the file
echo "=== Checking for type annotations ==="
rg ":\s*(int|str|bool|List|Dict|Optional|Any)" src/colony/base/system.py | head -10Repository: hivesolutions/colony
Length of output: 350
🏁 Script executed:
# Check the __init__ method to confirm logger initialization
echo "=== Checking PluginManager.__init__ ==="
awk '/^class PluginManager/,/^class [^(]/ {print NR": "$0}' src/colony/base/system.py | grep -A 50 "def __init__" | head -60Repository: hivesolutions/colony
Length of output: 103
🏁 Script executed:
# Verify black compatibility of proposed fix (check if code follows black style)
echo "=== Checking existing code style in nearby methods ==="
sed -n '6620,6650p' src/colony/base/system.pyRepository: hivesolutions/colony
Length of output: 968
🏁 Script executed:
# Test the proposed fix for Python 2.7 compatibility
python - <<'PY'
import logging
TRACE = logging.DEBUG - 5
# Test the proposed fix logic
class MockLogger:
def __init__(self):
self.level = logging.NOTSET
self.disabled = False
def isEnabledFor(self, level):
"""Mock standard logger isEnabledFor"""
if self.disabled:
return False
effective_level = self.level if self.level else logging.WARNING
return level >= effective_level
# Simulate the proposed fix
logger = MockLogger()
is_enabled_for = getattr(logger, "isEnabledFor", None)
if is_enabled_for:
result = is_enabled_for(TRACE)
else:
level = getattr(logger, "level", None)
if level == None:
result = False
else:
result = level <= TRACE
print("Mock logger result (NOTSET):", result)
# Test with actual logging
real_logger = logging.getLogger("test")
parent = logging.getLogger("test_parent")
parent.setLevel(TRACE)
real_logger.setLevel(logging.NOTSET)
is_enabled_for = getattr(real_logger, "isEnabledFor", None)
if is_enabled_for:
result = is_enabled_for(TRACE)
else:
level = getattr(real_logger, "level", None)
if level == None:
result = False
else:
result = level <= TRACE
print("Real logger result (inherited NOTSET):", result)
print("Real logger isEnabledFor(TRACE):", real_logger.isEnabledFor(TRACE))
PYRepository: hivesolutions/colony
Length of output: 182
🏁 Script executed:
# Verify the actual behavior difference more carefully with proper parent-child relationship
python - <<'PY'
import logging
TRACE = logging.DEBUG - 5
# Recreate the exact scenario from initial verification
parent = logging.getLogger("trace_parent")
child = logging.getLogger("trace_parent.child")
parent.setLevel(TRACE)
child.setLevel(logging.NOTSET)
print("=== Current implementation (direct .level check) ===")
# Simulate current is_trace implementation
if not child:
raw_result = False
elif not child.level:
raw_result = False
else:
raw_result = child.level <= TRACE
print(f"child.level = {child.level}")
print(f"child.logger.getEffectiveLevel() = {child.getEffectiveLevel()}")
print(f"Raw check result: {raw_result}")
print(f"isEnabledFor result: {child.isEnabledFor(TRACE)}")
print("\n=== With disabled logger ===")
child.disabled = True
if not child:
raw_result = False
elif not child.level:
raw_result = False
else:
raw_result = child.level <= TRACE
print(f"Raw check result: {raw_result}")
print(f"isEnabledFor result: {child.isEnabledFor(TRACE)}")
PYRepository: hivesolutions/colony
Length of output: 300
🏁 Script executed:
# Check line ending style more directly
echo "=== Checking line ending style ==="
hexdump -C src/colony/base/system.py | head -20Repository: hivesolutions/colony
Length of output: 161
🏁 Script executed:
# Verify the coding guideline: check for any existing "== None" vs "is None" patterns
echo "=== Checking existing None comparison style in the file ==="
rg "\s(==|is)\s+None" src/colony/base/system.py | head -20Repository: hivesolutions/colony
Length of output: 476
🏁 Script executed:
# Verify the proposed fix aligns with black formatter expectations
echo "=== Checking the proposed fix formatting ==="
python - <<'PY'
import logging
# Simulate the proposed fix to ensure it's syntactically valid Python 2.7+
def is_trace(logger, TRACE):
if not logger:
return False
is_enabled_for = getattr(logger, "isEnabledFor", None)
if is_enabled_for:
return is_enabled_for(TRACE)
level = getattr(logger, "level", None)
if level == None:
return False
return level <= TRACE
# Test the proposed fix
test_logger = logging.getLogger("test")
parent = logging.getLogger("test_parent")
parent.setLevel(logging.DEBUG - 5)
test_logger.setLevel(logging.NOTSET)
TRACE = logging.DEBUG - 5
result = is_trace(test_logger, TRACE)
print("Proposed fix result:", result)
print("Correct (should be True):", result == True)
PYRepository: hivesolutions/colony
Length of output: 167
🏁 Script executed:
# Check if there are any type annotations in the method signature
echo "=== Checking method signature (no type annotations required) ==="
sed -n '6596,6600p' src/colony/base/system.pyRepository: hivesolutions/colony
Length of output: 342
🏁 Script executed:
# Verify the proposed fix more carefully with proper logger hierarchy
python - <<'PY'
import logging
TRACE = logging.DEBUG - 5
# Set up the exact same hierarchy as the verification
parent = logging.getLogger("trace_parent")
child = logging.getLogger("trace_parent.child")
parent.setLevel(TRACE)
child.setLevel(logging.NOTSET)
# Simulate the proposed fix
logger = child
if not logger:
result = False
else:
is_enabled_for = getattr(logger, "isEnabledFor", None)
if is_enabled_for:
result = is_enabled_for(TRACE)
else:
level = getattr(logger, "level", None)
if level == None:
result = False
else:
result = level <= TRACE
print("Proposed fix result:", result)
print("Expected (True):", result == True)
print()
print("Verification: isEnabledFor(TRACE) =", child.isEnabledFor(TRACE))
print("Verification: child.level =", child.level)
print("Verification: child.getEffectiveLevel() =", child.getEffectiveLevel())
PYRepository: hivesolutions/colony
Length of output: 227
🏁 Script executed:
# Verify the proposed fix handles disabled loggers correctly
python - <<'PY'
import logging
TRACE = logging.DEBUG - 5
parent = logging.getLogger("trace_parent")
child = logging.getLogger("trace_parent.child")
parent.setLevel(TRACE)
child.setLevel(logging.NOTSET)
child.disabled = True
# Simulate the proposed fix with disabled logger
logger = child
if not logger:
result = False
else:
is_enabled_for = getattr(logger, "isEnabledFor", None)
if is_enabled_for:
result = is_enabled_for(TRACE)
else:
level = getattr(logger, "level", None)
if level == None:
result = False
else:
result = level <= TRACE
print("Proposed fix result (disabled):", result)
print("Expected (False):", result == False)
print("Verification: isEnabledFor(TRACE) =", child.isEnabledFor(TRACE))
PYRepository: hivesolutions/colony
Length of output: 166
Use effective-level semantics for is_trace().
The current implementation reads self.logger.level directly, which returns 0 (NOTSET) for loggers that inherit their effective level from parent loggers. This causes is_trace() to incorrectly return False even when the effective level would enable trace logging. Use isEnabledFor(loggers.TRACE) to respect the standard logging semantics for level inheritance and disabled loggers.
🔧 Prefer the logger API
if not self.logger:
return False
- if not self.logger.level:
- return False
- return self.logger.level <= loggers.TRACE
+ is_enabled_for = getattr(self.logger, "isEnabledFor", None)
+ if is_enabled_for:
+ return is_enabled_for(loggers.TRACE)
+ level = getattr(self.logger, "level", None)
+ if level == None:
+ return False
+ return level <= loggers.TRACE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/colony/base/system.py` around lines 6596 - 6611, The is_trace() method
incorrectly inspects self.logger.level directly (which may be 0/NOTSET for
inherited levels); change it to use the logger API so it respects
effective-level semantics: verify self.logger is not None and then return
self.logger.isEnabledFor(loggers.TRACE) (or False if no logger) so trace checks
honor parent/inherited levels; update references to is_trace, self.logger and
loggers.TRACE accordingly.
When the log level is set to TRACE, automatically switches to a more detailed format that includes file path and line number for fine-grained debugging of low-level operations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The GLOBAL_CONFIG always has a hardcoded logging_format value, so the .get() default never triggers. Added explicit check to switch to the trace format when the config value matches the default and the level is TRACE. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/colony/base/system.py (1)
6123-6138:⚠️ Potential issue | 🟡 MinorAdd missing
self.loggerguard incritical().Unlike debug/info/warning/error,
critical()now calls intoself.loggerunguarded. At Line 6135 this can raise when logger initialization is absent.💡 Suggested fix
def critical(self, message, *args, **kwargs): """ Adds the given critical message to the logger. @@ + # in case no logger is defined it's not possible + # to print the message as a critical + if not self.logger: + return + # formats the critical message logger_message = self.format_logger_message(message)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/colony/base/system.py` around lines 6123 - 6138, The critical() method calls self.logger.critical without checking self.logger; add the same guard used in debug/info/warning/error: ensure self.logger exists before formatting/logging by wrapping the call to self.format_logger_message(message) and self.logger.critical(...) inside an if self.logger: block (or early return if missing) and still call self.log_stack_trace(level=logging.ERROR) only as intended; update the critical() implementation to mirror the logger-presence checks used in the other level methods to avoid AttributeError when the logger is not initialized.
♻️ Duplicate comments (2)
src/colony/base/system.py (2)
6027-6044:⚠️ Potential issue | 🔴 CriticalUse
self.logger.trace(...)intrace()for logger compatibility.At Line 6043, dispatching via
self.logger.log(...)can fail for logger implementations that providetrace()but not genericlog(). This diverges from sibling methods (debug/info/...) that use direct level methods.💡 Suggested fix
- self.logger.log(loggers.TRACE, logger_message, *args, **kwargs) + self.logger.trace(logger_message, *args, **kwargs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/colony/base/system.py` around lines 6027 - 6044, The trace() method currently calls self.logger.log(loggers.TRACE, ...), which can fail for logger implementations that expose trace() but not a generic log(); replace that call with self.logger.trace(logger_message, *args, **kwargs) in the trace method (keeping the prior logger_message = self.format_logger_message(message) and the early return if not self.logger) so it matches the sibling debug/info/... methods and ensures compatibility with loggers that only implement level-specific methods.
6611-6626:⚠️ Potential issue | 🟡 MinorUse effective-level semantics in
is_trace().At Line 6624-Line 6626, checking
self.logger.leveldirectly is incorrect for NOTSET/inherited levels and disabled loggers. Use logger API semantics (isEnabledFor) instead.💡 Suggested fix
if not self.logger: return False - if not self.logger.level: - return False - return self.logger.level <= loggers.TRACE + return self.logger.isEnabledFor(loggers.TRACE)#!/bin/bash set -euo pipefail python - <<'PY' import logging TRACE = logging.DEBUG - 5 parent = logging.getLogger("trace_parent") child = logging.getLogger("trace_parent.child") parent.setLevel(TRACE) child.setLevel(logging.NOTSET) direct_level_check = False if not child.level else child.level <= TRACE print("direct_level_check =", direct_level_check) # expected: False print("isEnabledFor =", child.isEnabledFor(TRACE)) # expected: True child.disabled = True print("isEnabledFor_disabled =", child.isEnabledFor(TRACE)) # expected: False PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/colony/base/system.py` around lines 6611 - 6626, The current is_trace() implementation incorrectly inspects self.logger.level directly; change it to use the logger API semantics by returning whether the logger is enabled for the TRACE level (use self.logger.isEnabledFor(loggers.TRACE)), and keep the existing safety check for self.logger being truthy (e.g., return False if not self.logger, otherwise return self.logger.isEnabledFor(loggers.TRACE)); this ensures NOTSET/inherited levels and disabled loggers are handled correctly in is_trace().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/colony/base/system.py`:
- Around line 6123-6138: The critical() method calls self.logger.critical
without checking self.logger; add the same guard used in
debug/info/warning/error: ensure self.logger exists before formatting/logging by
wrapping the call to self.format_logger_message(message) and
self.logger.critical(...) inside an if self.logger: block (or early return if
missing) and still call self.log_stack_trace(level=logging.ERROR) only as
intended; update the critical() implementation to mirror the logger-presence
checks used in the other level methods to avoid AttributeError when the logger
is not initialized.
---
Duplicate comments:
In `@src/colony/base/system.py`:
- Around line 6027-6044: The trace() method currently calls
self.logger.log(loggers.TRACE, ...), which can fail for logger implementations
that expose trace() but not a generic log(); replace that call with
self.logger.trace(logger_message, *args, **kwargs) in the trace method (keeping
the prior logger_message = self.format_logger_message(message) and the early
return if not self.logger) so it matches the sibling debug/info/... methods and
ensures compatibility with loggers that only implement level-specific methods.
- Around line 6611-6626: The current is_trace() implementation incorrectly
inspects self.logger.level directly; change it to use the logger API semantics
by returning whether the logger is enabled for the TRACE level (use
self.logger.isEnabledFor(loggers.TRACE)), and keep the existing safety check for
self.logger being truthy (e.g., return False if not self.logger, otherwise
return self.logger.isEnabledFor(loggers.TRACE)); this ensures NOTSET/inherited
levels and disabled loggers are handled correctly in is_trace().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8c31d13a-c39f-4c11-bce3-bfcaa3037cd8
📒 Files selected for processing (3)
src/colony/__main__.pysrc/colony/base/system.pysrc/colony_adm.py
✅ Files skipped from review due to trivial changes (1)
- src/colony/main.py
Passes stacklevel=2 in all PluginManager log methods (trace, debug, info, warning, error, critical) so that pathname and lineno in the format string reflect the actual caller, not the internal log method. Guards stacklevel for Python < 3.8 where it is not supported, and makes DEFAULT_LOGGING_FORMAT_TRACE conditional on Python 3.8+. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
TRACEconstant (logging.DEBUG - 5 = 5) for fine-grained protocol-level debuggingSILENTconstant (logging.CRITICAL + 1 = 51) for silencing all loggingpatch_logging()to register the TRACE level with Python's logging (idempotent, called automatically instart_logger())is_trace()method toPluginManagertrace()toDummyLogger_levelNames/_levelValues/getLevelInt()soLEVEL=TRACEconfiguration worksTest plan
python -m unittest colony.test.base.loggers -v)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation