Promote internal LLM/session helpers to explicit public API; remove underscore re-exports from __init__.py - #70
Conversation
Agent-Logs-Url: https://github.com/CyberSecDef/NovelForge/sessions/153f8689-508a-4211-82f7-0846ee2d0e12 Co-authored-by: CyberSecDef <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR makes previously underscore-prefixed LLM/session helpers an explicit public API and removes underscore re-exports from package __init__.py files to prevent internal details from becoming accidental stable interfaces.
Changes:
- Promotes LLM helpers to public names (
reset_llm_usage,get_llm_usage,friendly_llm_error) and addsreset_circuit_breakers(). - Promotes session persistence helper to public name (
persist_completed_chapters) and stops exporting internal schema/constants. - Updates routes and tests to use the new public names.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
novelforge/llm/client.py |
Renames usage/error helpers and introduces reset_circuit_breakers() |
novelforge/llm/__init__.py |
Stops re-exporting underscore-prefixed internals; exports new public helpers |
novelforge/session/persistence.py |
Renames _persist_completed_chapters to persist_completed_chapters |
novelforge/session/__init__.py |
Removes underscore exports; exports persist_completed_chapters |
novelforge/routes/generation.py |
Migrates callers to new LLM/session helper names |
novelforge/routes/outline.py |
Uses friendly_llm_error instead of underscore variant |
novelforge/routes/export.py |
Uses friendly_llm_error instead of underscore variant |
tests/conftest.py |
Uses reset_circuit_breakers() instead of iterating internal dict |
tests/test_coverage.py |
Updates imports for renamed LLM helpers |
tests/test_session.py |
Updates imports/docs for renamed persistence helper |
Comments suppressed due to low confidence (1)
novelforge/llm/client.py:520
friendly_llm_error()is now part of the explicit public API and is being called with non-RuntimeErrorexceptions (e.g.,requests.exceptions.RequestExceptioninroutes/generation.py). The docstring currently says it converts aRuntimeErrorfromcall_llm, which is misleading; please update it to describe the broader intended input (any LLM/request/parsing exception) and the tagging/stripping behavior.
def friendly_llm_error(exc: Exception) -> str:
"""
Convert a RuntimeError from call_llm into a user-friendly message.
The error string from call_llm is prefixed with a tag like LLM_TIMEOUT:
which is stripped here. If no known tag is found, a generic message is used.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _friendly_llm_error(exc: Exception) -> str: | ||
| def reset_circuit_breakers() -> None: | ||
| """Reset all per-provider LLM circuit breakers.""" | ||
| for cb in _llm_circuit_breakers.values(): |
There was a problem hiding this comment.
reset_circuit_breakers() iterates over _llm_circuit_breakers.values() without holding _llm_cb_lock. Since _get_circuit_breaker() can add new entries concurrently, this risks RuntimeError: dictionary changed size during iteration (and partially-reset state). Consider acquiring _llm_cb_lock and iterating over a snapshot (e.g., list(_llm_circuit_breakers.values())) before calling reset() on each breaker.
| for cb in _llm_circuit_breakers.values(): | |
| with _llm_cb_lock: | |
| breakers = list(_llm_circuit_breakers.values()) | |
| for cb in breakers: |
Package
__init__.pyfiles were re-exporting underscore-prefixed names, making implementation details accidentally part of the stable public API and blurring the package boundary.Changes
novelforge/llm/client.py_reset_llm_usage→reset_llm_usage,_get_llm_usage→get_llm_usage,_friendly_llm_error→friendly_llm_errorreset_circuit_breakers()— encapsulates the previously leaked_llm_circuit_breakersiteration patternnovelforge/llm/__init__.py_reset_llm_usage,_get_llm_usage,_friendly_llm_error,_llm_circuit_breaker,_llm_circuit_breakers,_get_circuit_breaker)novelforge/session/persistence.py_persist_completed_chapters→persist_completed_chaptersnovelforge/session/__init__.py_SESSION_SCHEMA(used only insidepersistence.py) and_persist_completed_chapterspersist_completed_chaptersCallers updated —
routes/generation.py,routes/export.py,routes/outline.py,tests/conftest.py,tests/test_coverage.py,tests/test_session.pyBefore / After
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
api.openai.com/usr/bin/python python -m pytest tests/ --tb=short -q(dns block)If you need me to access, download, or install something from one of these locations, you can either: