Conversation
📝 WalkthroughWalkthroughThe change adds task-local context propagation across synchronous, asynchronous, threaded, and plugin execution paths. Public download callbacks are removed, exception options become keyword-only, logs include task identifiers, the deprecated CLI shim is removed, and documentation and tests are updated. ChangesTask Context and Download APIs
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant DownloadAPI
participant Executor
participant Logger
Caller->>DownloadAPI: start download
DownloadAPI->>DownloadAPI: set jm_task_context
DownloadAPI->>Executor: run bound task
Executor-->>Logger: emit contextual record
Logger-->>Caller: formatted task-aware log
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Clarify callback and batch exception contracts, add task-scoped logging across sync and async workers, and update the mobile API version.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@assets/docs/sources/tutorial/11_log_custom.md`:
- Line 117: Update the formatter assignment in the custom logging example to
avoid indexing jm_logger.handlers when no handlers are configured. Reuse the
first handler’s formatter when available, and otherwise apply a valid formatter
fallback before registering TaskLogHandler.
In `@assets/docs/sources/tutorial/4_module_custom.md`:
- Around line 20-22: Update the after_album override in the tutorial example to
delegate to JmDownloader.after_album by calling super().after_album(album),
while retaining the existing custom print behavior and choosing an appropriate
call order.
In `@README.md`:
- Line 241: Update the README bullet describing download event callbacks to
state that users must customize the behavior through a custom Downloader, such
as by subclassing or configuring it, rather than implying a public callback
parameter.
In `@src/jmcomic/__init__.py`:
- Line 7: Replace the star import from jm_task_context in the package
initializer with an explicit import of JM_TASK_CONTEXT, jm_task_context,
get_jm_task_context, and bind_jm_task_context, preserving the existing
package-level API.
In `@src/jmcomic/jm_task_context.py`:
- Around line 41-45: Update bind_jm_task_context to validate func with
callable(func) before async detection or wrapping, rejecting non-callable
targets immediately. Retain the existing __call__ inspection only for
identifying asynchronous callable objects, and preserve the current TypeError
behavior for unsupported async callables.
🪄 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 Plus
Run ID: 62f4de3f-407c-4dcd-b46a-c5266f1923e5
📒 Files selected for processing (18)
README.mdassets/docs/sources/option_file_syntax.mdassets/docs/sources/tutorial/0_common_usage.mdassets/docs/sources/tutorial/11_log_custom.mdassets/docs/sources/tutorial/13_export_and_feature.mdassets/docs/sources/tutorial/4_module_custom.mdsrc/jmcomic/__init__.pysrc/jmcomic/api.pysrc/jmcomic/jm_async_downloader.pysrc/jmcomic/jm_client_impl.pysrc/jmcomic/jm_config.pysrc/jmcomic/jm_downloader.pysrc/jmcomic/jm_feature.pysrc/jmcomic/jm_plugin.pysrc/jmcomic/jm_task_context.pytests/test_jmcomic/test_jm_api.pytests/test_jmcomic/test_jm_async_api.pytests/test_jmcomic/test_jm_task_context.py
💤 Files with no reviewable changes (1)
- tests/test_jmcomic/test_jm_async_api.py
|
|
||
|
|
||
| handler = TaskLogHandler() | ||
| handler.setFormatter(jm_logger.handlers[0].formatter) # 复用jmcomic默认 Handler 的日志格式，你也可以自定义日志格式 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Avoid assuming a default handler exists.
This example indexes jm_logger.handlers[0], but the customization guide also supports clearing or replacing handlers. With no handlers configured, the example raises IndexError before registering TaskLogHandler; use a formatter fallback instead.
Proposed fallback
handler = TaskLogHandler()
-handler.setFormatter(jm_logger.handlers[0].formatter)
+formatter = next(
+ (item.formatter for item in jm_logger.handlers if item.formatter is not None),
+ logging.Formatter('%(message)s'),
+)
+handler.setFormatter(formatter)
jm_logger.addHandler(handler)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handler.setFormatter(jm_logger.handlers[0].formatter) # 复用jmcomic默认 Handler 的日志格式，你也可以自定义日志格式 | |
| formatter = next( | |
| (item.formatter for item in jm_logger.handlers if item.formatter is not None), | |
| logging.Formatter('%(message)s'), | |
| ) | |
| handler.setFormatter(formatter) |
🤖 Prompt for 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.
In `@assets/docs/sources/tutorial/11_log_custom.md` at line 117, Update the
formatter assignment in the custom logging example to avoid indexing
jm_logger.handlers when no handlers are configured. Reuse the first handler’s
formatter when available, and otherwise apply a valid formatter fallback before
registering TaskLogHandler.
| def after_album(self, album: JmAlbumDetail): | ||
| print(f'album下载完毕: {album}') | ||
| pass |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the base after-album lifecycle.
This override replaces JmDownloader.after_album and never delegates to it. Users copying the example can therefore disable the built-in after-album plugin/Feature pipeline. Call super().after_album(album) before or after the custom behavior, depending on the desired ordering.
Proposed fix
def after_album(self, album: JmAlbumDetail):
+ super().after_album(album)
print(f'album下载完毕: {album}')
- pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def after_album(self, album: JmAlbumDetail): | |
| print(f'album下载完毕: {album}') | |
| pass | |
| def after_album(self, album: JmAlbumDetail): | |
| super().after_album(album) | |
| print(f'album下载完毕: {album}') |
🤖 Prompt for 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.
In `@assets/docs/sources/tutorial/4_module_custom.md` around lines 20 - 22, Update
the after_album override in the tutorial example to delegate to
JmDownloader.after_album by calling super().after_album(album), while retaining
the existing custom print behavior and choosing an appropriate call order.
| - **可扩展性强** | ||
|
|
||
| - 支持自定义本子/章节/图片下载前后的回调函数 | ||
| - 支持自定义本子/章节/图片下载前后事件的回调函数 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify that download events require a custom Downloader.
This bullet still suggests that users can customize download callbacks directly, while the updated API contract removes the public callback parameter. State explicitly that event behavior is customized by subclassing/configuring Downloader.
Proposed wording
- - 支持自定义本子/章节/图片下载前后事件的回调函数
+ - 支持通过自定义 Downloader 响应本子/章节/图片下载前后事件（公开下载 API 不接收 callback 参数）📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - 支持自定义本子/章节/图片下载前后事件的回调函数 | |
| - 支持通过自定义 Downloader 响应本子/章节/图片下载前后事件（公开下载 API 不接收 callback 参数） |
🤖 Prompt for 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.
In `@README.md` at line 241, Update the README bullet describing download event
callbacks to state that users must customize the behavior through a custom
Downloader, such as by subclassing or configuring it, rather than implying a
public callback parameter.
|
|
||
| __version__ = '2.7.2' | ||
|
|
||
| from .jm_task_context import * |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)__init__\.py$|jm_task_context'
echo "== __init__ around line =="
cat -n src/jmcomic/__init__.py | sed -n '1,80p'
echo "== jm_task_context outline =="
ast-grep outline src/jmcomic/jm_task_context.py 2>/dev/null || true
echo "== jm_task_context relevant symbols =="
rg -n "^(JM_TASK_CONTEXT|bind_jm_task_context|get_jm_task_context|jm_task_context)\b|__all__" src/jmcomic/jm_task_context.py
echo "== imports of jm_task_context from package root / globals =="
rg -n "from \. jm_task_context import \*|from \.jm_task_context import|get_jm_task_context|JM_TASK_CONTEXT|bind_jm_task_context|jm_task_context" src/jmcomic/__init__.py src/jmcomic/jm_task_context.pyRepository: hect0x7/JMComic-Crawler-Python
Length of output: 3802
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file listing =="
git ls-files | grep -E '(^|/)__init__\.py$|jm_task_context|setup|pyproject|poetry|requirements' || true
echo "== __init__ =="
cat -n src/jmcomic/__init__.py | sed -n '1,120p'
echo "== task context relevant =="
rg -n "^(def |class |JM_TASK_CONTEXT|bind_jm_task_context|get_jm_task_context|jm_task_context|__all__|= .*Task.*|= .*Context.*|@)" src/jmcomic/jm_task_context.pyRepository: hect0x7/JMComic-Crawler-Python
Length of output: 2267
Replace the task-context star import with explicit re-exports.
jm_task_context.__all__ exposes JM_TASK_CONTEXT, jm_task_context, get_jm_task_context, and bind_jm_task_context; importing those names explicitly keeps the same package-level API and avoids F403/static-analysis ambiguity.
Proposed fix
-from .jm_task_context import *
+from .jm_task_context import (
+ JM_TASK_CONTEXT,
+ bind_jm_task_context,
+ get_jm_task_context,
+ jm_task_context,
+)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from .jm_task_context import * | |
| from .jm_task_context import ( | |
| JM_TASK_CONTEXT, | |
| bind_jm_task_context, | |
| get_jm_task_context, | |
| jm_task_context, | |
| ) |
🧰 Tools
🪛 Ruff (0.16.0)
[error] 7-7: from .jm_task_context import * used; unable to detect undefined names
(F403)
🤖 Prompt for 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.
In `@src/jmcomic/__init__.py` at line 7, Replace the star import from
jm_task_context in the package initializer with an explicit import of
JM_TASK_CONTEXT, jm_task_context, get_jm_task_context, and bind_jm_task_context,
preserving the existing package-level API.
Source: Linters/SAST tools
| def bind_jm_task_context(func: Callable, context: Optional[Mapping] = None) -> Callable: | ||
| """Bind a synchronous callable to a snapshot of the current task context.""" | ||
| if (inspect.iscoroutinefunction(func) | ||
| or inspect.iscoroutinefunction(getattr(func, '__call__', None))): | ||
| raise TypeError('bind_jm_task_context only supports synchronous callables') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file and its context.
fd -a 'jm_task_context.py' . || true
file="$(fd 'jm_task_context.py' . | head -n 1 || true)"
if [ -n "${file:-}" ]; then
echo "== file: $file =="
wc -l "$file"
echo
sed -n '1,180p' "$file" | cat -n
fi
echo
echo "== references to bind_jm_task_context =="
rg -n "bind_jm_task_context|call_async|executor" . --glob '!**/.git/**' || true
echo
echo "== Python probe: current callable/check shape without repository execution =="
python3 - <<'PY'
import inspect
import types
def bind_jm_task_context_current(func, context=None):
if (inspect.iscoroutinefunction(func)
or inspect.iscoroutinefunction(getattr(func, '__call__', None))):
raise TypeError('bind_jm_task_context only supports synchronous callables')
def wrapped(*a, **kw):
print("wrapped executed with", a, kw)
return wrapped
def bound_func():
return None
class AsyncCallable:
async def __call__(self, value):
return value
cases = [None, 123, [], lambda: None, bound_func, AsyncCallable()]
for case in cases:
try:
wrapped = bind_jm_task_context_current(case)
print(type(case).__name__, "bound without error")
try:
wrapped()
except Exception as e:
print(" execution failure:", type(e).__name__, str(e))
except Exception as e:
print(type(case).__name__, "binding failure:", type(e).__name__, str(e))
PYRepository: hect0x7/JMComic-Crawler-Python
Length of output: 7846
Fail fast for non-callable targets.
bind_jm_task_context currently accepts None and other non-callables, which only fail when wrapped invokes func. Add a callable(func) guard before accepting the target, while keeping the __call__ inspection only for detecting async callable objects.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 44-44: Using hasattr(x, "__call__") to test if x is callable is unreliable. Use callable(x) for consistent results.
(B004)
🤖 Prompt for 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.
In `@src/jmcomic/jm_task_context.py` around lines 41 - 45, Update
bind_jm_task_context to validate func with callable(func) before async detection
or wrapping, rejecting non-callable targets immediately. Retain the existing
__call__ inspection only for identifying asynchronous callable objects, and
preserve the current TypeError behavior for unsupported async callables.
Source: Linters/SAST tools
There was a problem hiding this comment.
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 `@src/jmcomic/jm_config.py`:
- Line 612: Update the docstring for the ANSI-colored log formatter to replace
the fullwidth comma with an ASCII comma, preserving the existing wording and
behavior so Ruff’s RUF002 check passes.
🪄 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 Plus
Run ID: e859f80d-a09f-4951-b6c7-450765bd029c
📒 Files selected for processing (18)
README.mdassets/docs/sources/option_file_syntax.mdassets/docs/sources/tutorial/0_common_usage.mdassets/docs/sources/tutorial/11_log_custom.mdassets/docs/sources/tutorial/13_export_and_feature.mdassets/docs/sources/tutorial/4_module_custom.mdsrc/jmcomic/__init__.pysrc/jmcomic/api.pysrc/jmcomic/jm_async_downloader.pysrc/jmcomic/jm_client_impl.pysrc/jmcomic/jm_config.pysrc/jmcomic/jm_downloader.pysrc/jmcomic/jm_feature.pysrc/jmcomic/jm_plugin.pysrc/jmcomic/jm_task_context.pytests/test_jmcomic/test_jm_api.pytests/test_jmcomic/test_jm_async_api.pytests/test_jmcomic/test_jm_task_context.py
💤 Files with no reviewable changes (1)
- tests/test_jmcomic/test_jm_async_api.py
🚧 Files skipped from review as they are similar to previous changes (11)
- README.md
- src/jmcomic/jm_plugin.py
- src/jmcomic/jm_feature.py
- src/jmcomic/jm_client_impl.py
- src/jmcomic/jm_downloader.py
- assets/docs/sources/option_file_syntax.md
- assets/docs/sources/tutorial/13_export_and_feature.md
- assets/docs/sources/tutorial/0_common_usage.md
- assets/docs/sources/tutorial/4_module_custom.md
- assets/docs/sources/tutorial/11_log_custom.md
- src/jmcomic/jm_async_downloader.py
There was a problem hiding this comment.
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 `@src/jmcomic/jm_config.py`:
- Line 23: Update the comment near the task_context logging logic in
jm_config.py by replacing both fullwidth commas with ASCII commas, resolving the
remaining RUF003 lint violation without changing the code behavior.
🪄 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 Plus
Run ID: ac8cb86d-a8a0-47ed-ac2b-4a2924e8179b
📒 Files selected for processing (6)
assets/docs/sources/api/command-line.mdsrc/jmcomic/cl.pysrc/jmcomic/jm_config.pytests/test_jmcomic/test_jm_cli.pytests/test_jmcomic/test_jm_task_context.pyusage/workflow_download.py
💤 Files with no reviewable changes (1)
- src/jmcomic/cl.py
| fields = [] | ||
| task_id = context.get('task_id') | ||
| if task_id is None: | ||
| # 没有 task_id 时，默认不打印 task_context，保持日志简洁 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the remaining RUF003 lint violation.
Line 23 contains two fullwidth commas; replace them with ASCII commas so lint passes.
Proposed fix
- # 没有 task_id 时，默认不打印 task_context，保持日志简洁
+ # 没有 task_id 时, 默认不打印 task_context, 保持日志简洁📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 没有 task_id 时，默认不打印 task_context，保持日志简洁 | |
| # 没有 task_id 时, 默认不打印 task_context, 保持日志简洁 |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 23-23: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
[warning] 23-23: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 Prompt for 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.
In `@src/jmcomic/jm_config.py` at line 23, Update the comment near the
task_context logging logic in jm_config.py by replacing both fullwidth commas
with ASCII commas, resolving the remaining RUF003 lint violation without
changing the code behavior.
Source: Linters/SAST tools
1 participant
Clarify callback and batch exception contracts, add task-scoped logging across sync and async workers, and update the mobile API version.
Summary by CodeRabbit
callbackparameters from single-album/single-photo download APIs (sync/async);check_exceptionis now keyword-only.