refactor(ai): replace print statements with structured logging#155
Conversation
|
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:
WalkthroughAdded module-level logging and replaced print statements with logger calls in AI modules, annotated ModelManager.list_models return type, and gated Rust CI steps behind a change-detection step in the GitHub Actions workflow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for opening your first pull request in NexumDB!
We appreciate your contribution! Here's what happens next:
- ✅ Automated checks will run (CI, tests, linting)
- 👀 A maintainer will review your changes
- 💬 We may suggest some improvements or ask questions
- 🚀 Once approved, we'll merge your contribution!
Before we can merge:
- Make sure all CI checks pass
- Sign your commits with DCO (
git commit -s) - Address any review feedback
Check out our Contributing Guide for more details.
Thanks for making NexumDB better! 🙌
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nexum_ai/model_manager.py (1)
128-129:⚠️ Potential issue | 🟠 MajorSame issue as
translator.py:__main__block lackslogging.basicConfig().Running
python -m nexum_ai.model_managerwill produce no visible output because no handler is configured.Proposed fix
if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) test_model_manager()
🤖 Fix all issues with AI agents
In `@nexum_ai/model_manager.py`:
- Line 26: The ModelManager logs its filesystem path at INFO level via
logger.info(f"ModelManager initialized with directory: {self.models_dir}"),
which exposes directory info; change this to logger.debug so the routine
lifecycle message and the models_dir attribute are only visible in debug logs
(update the call in the ModelManager initialization/constructor where models_dir
is referenced).
- Around line 93-95: The except block currently calls logger.exception("Error
downloading model") then a separate logger.info hint; remove the separate
logger.info and instead include the user-facing hint in the logger.exception
call so the error and the hint are a single ERROR-level entry (e.g., change the
logger.exception message to include "Please check your internet connection and
HuggingFace credentials"); update the except block around the model download
code (the logger.exception call in model_manager.py) accordingly so only the
enriched logger.exception is emitted.
- Line 98: The public method list_models is missing a return type annotation;
inspect what it actually returns and add an explicit return type on def
list_models (for example -> list[str] or -> list[Model] as appropriate), and
update imports if needed (from typing import List) or use built-in list[...]
syntax to match the rest of the file's type-hint style so the signature is
consistent with other public methods.
- Line 121: The log calls in model_manager.py use incorrect severity: change the
"No models found" call in the code path that logs currently with logger.error
(around the block that checks for downloaded models—look for the message " No
models found") to logger.info, and change the earlier directory-missing
logger.error (the check near the __init__ created directory, around the code
that verifies the models directory existence) to logger.warning; update the two
logger.* invocations accordingly so they reflect informational and warning
semantics instead of errors.
In `@nexum_ai/translator.py`:
- Line 53: The log statement uses logger.info to print the full filesystem path
(logger.info(f"Loading LLM from {model_path}...")), which can expose sensitive
path details; change this to a lower verbosity level and avoid full-path
exposure by using logger.debug and/or logging only the model base name or an
anonymized identifier (e.g., os.path.basename(model_path) or
"model_path=<redacted>") wherever the Loading LLM log is emitted so replace the
logger.info call with a logger.debug that does not include the raw model_path.
- Line 95: The log currently emits raw user input and generated SQL at INFO
level via logger.info with natural_query and sql, which can leak sensitive data;
change this call to logger.debug (or otherwise lower the log level) so the
translated content is only logged in debug mode, updating the logger statement
that references natural_query and sql (the translation/logging site) to use
debug-level logging and ensure any future similar logging of user_input/sql uses
debug rather than info.
- Around line 61-63: Remove the extraneous space before the colon in the except
clauses and bind the exception to a variable; replace occurrences like the block
containing logger.exception("Could not load LLM") and self.model = None (and the
similar block around Line 98) with an except clause that binds the exception
(e.g., except Exception as e:) so the intent is clear and the exception can be
referenced if needed while still calling logger.exception.
- Around line 190-195: The test helper currently uses logger.info for output
(see logger.info calls and translator.translate inside test_translator()), but
logging is never configured so output is dropped when running via if __name__ ==
"__main__":; fix by configuring the logging subsystem in the module’s main block
(e.g., call logging.basicConfig(level=logging.INFO) before invoking
test_translator()) or revert the test helper to use print() for visible,
user-facing output when executed as a script, ensuring the change is applied
around the if __name__ == "__main__": entrypoint that calls test_translator().
| else: | ||
| try: | ||
| print(f"Loading LLM from {model_path}...") | ||
| logger.info(f"Loading LLM from {model_path}...") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Logging model file path at INFO level may expose filesystem details.
logger.info(f"Loading LLM from {model_path}...") reveals the full filesystem path to any log consumer. This is low risk but worth noting — DEBUG level would be more appropriate for operational paths.
🤖 Prompt for AI Agents
In `@nexum_ai/translator.py` at line 53, The log statement uses logger.info to
print the full filesystem path (logger.info(f"Loading LLM from
{model_path}...")), which can expose sensitive path details; change this to a
lower verbosity level and avoid full-path exposure by using logger.debug and/or
logging only the model base name or an anonymized identifier (e.g.,
os.path.basename(model_path) or "model_path=<redacted>") wherever the Loading
LLM log is emitted so replace the logger.info call with a logger.debug that does
not include the raw model_path.
| except Exception : | ||
| logger.exception("Could not load LLM") | ||
| self.model = None |
There was a problem hiding this comment.
PEP 8: extraneous whitespace before : in except clause.
except Exception : has a space before the colon. This is flagged by PEP 8 (E203). Also, consider binding the exception variable even if you rely on logger.exception — it makes the intent clearer and avoids confusion with a bare except.
The same issue appears on Line 98.
Proposed fix
- except Exception :
- logger.exception("Could not load LLM")
+ except Exception:
+ logger.exception("Could not load LLM")And at Line 98:
- except Exception :
- logger.exception("Translation error")
+ except Exception:
+ logger.exception("Translation error")🤖 Prompt for AI Agents
In `@nexum_ai/translator.py` around lines 61 - 63, Remove the extraneous space
before the colon in the except clauses and bind the exception to a variable;
replace occurrences like the block containing logger.exception("Could not load
LLM") and self.model = None (and the similar block around Line 98) with an
except clause that binds the exception (e.g., except Exception as e:) so the
intent is clear and the exception can be referenced if needed while still
calling logger.exception.
Subhayoni
changed the title
Subhayoni
changed the title
aviralgarg05
left a comment
aviralgarg05
left a comment
There was a problem hiding this comment.
Resolve the issues by coderabbit, rest it is perfect
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
nexum_ai/model_manager.py (2)
130-131:⚠️ Potential issue | 🟡 MinorMissing
logging.basicConfigin__main__block — test output will be silently dropped.Unlike
translator.py(line 199), this file's__main__guard doesn't configure logging, so runningpython model_manager.pydirectly produces no visible output.Proposed fix
if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) test_model_manager()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nexum_ai/model_manager.py` around lines 130 - 131, The __main__ guard currently just calls test_model_manager() so any logs may be dropped; update the block that contains the __name__ == "__main__" check to configure Python logging first (e.g., call logging.basicConfig with an appropriate level and format) before invoking test_model_manager(); locate the __main__ section and add the logging.basicConfig(...) call so test_model_manager() logs are emitted.
75-80:⚠️ Potential issue | 🟠 MajorRemove
local_dir_use_symlinks=Falseparameter.The
local_dir_use_symlinksparameter was deprecated and removed fromhf_hub_download()inhuggingface_hubv1.0. Passing it will raise aTypeError. In earlier versions, it triggers a deprecation warning. Simply remove the parameter; downloads tolocal_dirno longer use symlinks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nexum_ai/model_manager.py` around lines 75 - 80, In model_manager.py remove the deprecated local_dir_use_symlinks=False argument from the hf_hub_download(...) call so it no longer passes that parameter; locate the hf_hub_download invocation that assigns downloaded_path (and references self.models_dir) and call hf_hub_download with the remaining valid params (repo_id, filename, local_dir) to avoid the TypeError from newer huggingface_hub versions.nexum_ai/translator.py (2)
139-146:⚠️ Potential issue | 🟠 MajorSQL injection in fallback translation via string interpolation.
f"SELECT * FROM users WHERE name = '{name}'"directly interpolates unsanitized user input into SQL. Even though this is a "simple fallback," this query string is returned to callers (including the Rust bridge pernexum_core/src/bridge/mod.rs:186-195) and could be executed against a real database.At minimum, consider escaping or documenting that the fallback output must be treated as untrusted. A safer approach would be to return parameterized query placeholders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nexum_ai/translator.py` around lines 139 - 146, The fallback in translator.py that returns f"SELECT * FROM users WHERE name = '{name}'" interpolates unsanitized user input (the branch handling "all users" / "named"/"called"); change it to produce a parameterized query and separate params (e.g., "SELECT * FROM users WHERE name = ?" plus a params list/tuple containing name) or otherwise escape and mark as untrusted, and update the callers to expect and use the (query, params) shape instead of raw interpolated SQL so execution uses proper parameter binding rather than string interpolation.
116-131:⚠️ Potential issue | 🟡 Minor
_clean_sqlsilently truncates multi-statement output at the first semicolon.Line 129 (
sql = sql.split(";")[0]) silently drops everything after the first;. Given that the linked issue#142requires explicit errors for multi-statement input, this silent truncation in the AI-generated path could mask problems. If the LLM generates multi-statement SQL, the caller would receive only the first statement with no indication that content was dropped. Consider logging a warning when truncation occurs.Proposed fix
- sql = sql.split(";")[0] + parts = sql.split(";") + if len(parts) > 1 and any(p.strip() for p in parts[1:]): + logger.warning("LLM generated multi-statement SQL; truncating to first statement") + sql = parts[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nexum_ai/translator.py` around lines 116 - 131, _clean_sql currently truncates at the first semicolon with sql = sql.split(";")[0], silently dropping additional statements; update _clean_sql to detect multi-statement output after normalizing (after removing leading "sql", backticks, and whitespace), and if any non-empty content remains after the first semicolon either log a clear warning and raise an explicit error (e.g., ValueError("LLM returned multi-statement SQL")) or at minimum log the original SQL and raise so callers are aware; reference the _clean_sql function and replace the simple split with logic that checks parts = sql.split(";"), trims each part, and reacts (warn+raise) when len(parts) > 1 and any trailing part is non-empty instead of silently returning only parts[0].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nexum_ai/translator.py`:
- Line 199: PEP8 violation in the call to logging.basicConfig: remove spaces
around the keyword assignment so the argument reads level=logging.INFO; update
the logging.basicConfig(...) invocation (symbol: logging.basicConfig) to use
level=logging.INFO instead of level = logging.INFO to comply with PEP 8 E251.
---
Outside diff comments:
In `@nexum_ai/model_manager.py`:
- Around line 130-131: The __main__ guard currently just calls
test_model_manager() so any logs may be dropped; update the block that contains
the __name__ == "__main__" check to configure Python logging first (e.g., call
logging.basicConfig with an appropriate level and format) before invoking
test_model_manager(); locate the __main__ section and add the
logging.basicConfig(...) call so test_model_manager() logs are emitted.
- Around line 75-80: In model_manager.py remove the deprecated
local_dir_use_symlinks=False argument from the hf_hub_download(...) call so it
no longer passes that parameter; locate the hf_hub_download invocation that
assigns downloaded_path (and references self.models_dir) and call
hf_hub_download with the remaining valid params (repo_id, filename, local_dir)
to avoid the TypeError from newer huggingface_hub versions.
In `@nexum_ai/translator.py`:
- Around line 139-146: The fallback in translator.py that returns f"SELECT *
FROM users WHERE name = '{name}'" interpolates unsanitized user input (the
branch handling "all users" / "named"/"called"); change it to produce a
parameterized query and separate params (e.g., "SELECT * FROM users WHERE name =
?" plus a params list/tuple containing name) or otherwise escape and mark as
untrusted, and update the callers to expect and use the (query, params) shape
instead of raw interpolated SQL so execution uses proper parameter binding
rather than string interpolation.
- Around line 116-131: _clean_sql currently truncates at the first semicolon
with sql = sql.split(";")[0], silently dropping additional statements; update
_clean_sql to detect multi-statement output after normalizing (after removing
leading "sql", backticks, and whitespace), and if any non-empty content remains
after the first semicolon either log a clear warning and raise an explicit error
(e.g., ValueError("LLM returned multi-statement SQL")) or at minimum log the
original SQL and raise so callers are aware; reference the _clean_sql function
and replace the simple split with logic that checks parts = sql.split(";"),
trims each part, and reacts (warn+raise) when len(parts) > 1 and any trailing
part is non-empty instead of silently returning only parts[0].
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nexum_ai/translator.py`:
- Around line 190-196: The demo helper test_translator currently uses
logger.info for user-facing messages (the block that logs "Natural Language
Translation Tests:", the separator, and each NL/SQL pair produced via
translator.translate), which can be silent if logging isn't configured; replace
those logger.info calls with print statements so output is always visible when
test_translator is run or imported and invoked (preserve the same messages and
formatting such as the "=" * 60 separator and the f"NL: ..." / f"SQL: ..."
lines).
---
Duplicate comments:
In `@nexum_ai/translator.py`:
- Around line 61-62: Replace the bare exception handlers with bound exception
variables so intent is explicit: change the two occurrences of "except
Exception:" that call logger.exception to "except Exception as e:" and keep
calling logger.exception (it will still include the traceback) or pass the bound
variable where appropriate; targets to update are the except blocks that call
logger.exception (the one currently logging "Could not load LLM" and the similar
handler around line 98).
| logger.info("Natural Language Translation Tests:") | ||
| logger.info("=" * 60) | ||
| for nl_query in test_cases: | ||
| sql = translator.translate(nl_query, schema) | ||
| print(f"NL: {nl_query}") | ||
| print(f"SQL: {sql}") | ||
| print() | ||
| logger.info(f"NL: {nl_query}") | ||
| logger.info(f"SQL: {sql}") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using print for user-facing demo output in test_translator.
logger.info calls inside test_translator will be silently swallowed if this function is ever called without prior logging.basicConfig. Since this is a demo/test helper (not library logic), print is arguably the right tool — it guarantees visible output regardless of logging configuration. The __main__ guard covers the direct-execution case, but an import-and-call scenario would produce no output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nexum_ai/translator.py` around lines 190 - 196, The demo helper
test_translator currently uses logger.info for user-facing messages (the block
that logs "Natural Language Translation Tests:", the separator, and each NL/SQL
pair produced via translator.translate), which can be silent if logging isn't
configured; replace those logger.info calls with print statements so output is
always visible when test_translator is run or imported and invoked (preserve the
same messages and formatting such as the "=" * 60 separator and the f"NL: ..." /
f"SQL: ..." lines).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nexum_ai/model_manager.py (2)
100-107: 🧹 Nitpick | 🔵 TrivialMinor type-hint style inconsistency.
list_modelsuses the built-inlist[str](3.9+ syntax), while the rest of the file usestyping.Optional. Consider aligning to one style — eitherOptional[str]everywhere orstr | None+list[str]everywhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nexum_ai/model_manager.py` around lines 100 - 107, The return type annotation on list_models uses the built-in generic syntax (def list_models(self) -> list[str]) which is inconsistent with the file's use of typing.*; change the annotation to use typing.List[str] (update imports if necessary) so the signature becomes def list_models(self) -> List[str] and ensure any other type annotations in this module follow the same typing.* style (e.g., typing.Optional) to keep the file consistent; reference: list_models and models_dir.
130-131:⚠️ Potential issue | 🟡 MinorMissing
logging.basicConfig()— running this file directly will produce no visible output.Without configuring a handler, Python's last-resort handler only emits
WARNING+ to stderr. All thelogger.info(...)calls intest_model_manager()will be silently dropped. The AI summary notes thattranslator.pyaddsbasicConfigin its__main__guard — this file should do the same for consistency and functionality.Proposed fix
if __name__ == "__main__": + logging.basicConfig(level=logging.DEBUG) test_model_manager()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nexum_ai/model_manager.py` around lines 130 - 131, The module's __main__ block calls test_model_manager() but doesn't configure logging, so INFO logs are dropped; add a logging.basicConfig(...) call (e.g., logging.basicConfig(level=logging.INFO, format=...)) in the __main__ guard before invoking test_model_manager() and ensure the logging module is imported/available so the logger.info(...) calls inside test_model_manager() produce visible output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nexum_ai/model_manager.py`:
- Line 26: Replace eager f-string interpolation in logger.debug calls with lazy
%-style formatting so logging formatting is deferred; specifically update the
ModelManager logger statements (e.g., the initialization log "ModelManager
initialized with directory: {self.models_dir}" and other debug calls like the
"Model found at {local_path}" and the third debug at line 84) to use
logger.debug("ModelManager initialized with directory: %s", self.models_dir) and
analogous forms (logger.debug("Model found at %s", local_path), etc.) wherever
logger.debug currently uses f-strings in the ModelManager class.
---
Outside diff comments:
In `@nexum_ai/model_manager.py`:
- Around line 100-107: The return type annotation on list_models uses the
built-in generic syntax (def list_models(self) -> list[str]) which is
inconsistent with the file's use of typing.*; change the annotation to use
typing.List[str] (update imports if necessary) so the signature becomes def
list_models(self) -> List[str] and ensure any other type annotations in this
module follow the same typing.* style (e.g., typing.Optional) to keep the file
consistent; reference: list_models and models_dir.
- Around line 130-131: The module's __main__ block calls test_model_manager()
but doesn't configure logging, so INFO logs are dropped; add a
logging.basicConfig(...) call (e.g., logging.basicConfig(level=logging.INFO,
format=...)) in the __main__ guard before invoking test_model_manager() and
ensure the logging module is imported/available so the logger.info(...) calls
inside test_model_manager() produce visible output.
aviralgarg05
left a comment
aviralgarg05
left a comment
There was a problem hiding this comment.
Fix the failing test and issues
Hi! I know the CI test is failing, but when i ran the python test locally, it passed on my side. I have not made changes to any rust code. Please let me know if there’s something specific I should check, or if I should sync with the latest base branch. Thanks! |
…t-with-logging # Conflicts: # .github/workflows/ci.yml
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 (1)
.github/workflows/ci.yml (1)
113-116:⚠️ Potential issue | 🟠 Major
benchmarksjob runs full Rust build for every PR, bypassing the new change detectionThe
rustjob's Rust-change gating is not applied to thebenchmarksjob. Becausebenchmarksis a separate job it cannot accesssteps.rust_changes.outputs. As a result, every PR — including Python-only ones — still triggers a fullcargo benchrun across all four suites, which is likely heavier than therustjob itself (especially sincebenchmarksusesfetch-depth: 0).To close this gap, add a parallel change-detection output at the job level and gate
benchmarkson it:🔧 Proposed fix — propagate rust_changes as a job output and gate benchmarks
jobs: rust: name: Rust fmt, clippy, tests runs-on: ubuntu-latest + outputs: + rust_changed: ${{ steps.rust_changes.outputs.changed }} env: ... benchmarks: name: Rust benchmarks runs-on: ubuntu-latest - if: github.event_name == 'pull_request' + needs: rust + if: github.event_name == 'pull_request' && needs.rust.outputs.rust_changed == 'true'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 113 - 116, The benchmarks job is missing the Rust-change gating because it can't read steps.rust_changes.outputs from the separate rust job; modify the rust job to expose a job output (e.g. add outputs: rust_changes: ${{ steps.rust_changes.outputs.rust_changes }}) and then gate the benchmarks job by referencing that job output (add needs: rust and if: needs.rust.outputs.rust_changes == 'true' on the benchmarks job). Update the benchmarks job to keep its runs-on/fetch-depth but only run when the exported rust_changes job output indicates Rust files changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 96-97: The "Upload coverage to Codecov" step uses an inconsistent
if expression with explicit interpolation (${ { ... } }) — change the if:
expression to the bare GitHub Actions expression syntax to match other gated
steps (i.e., replace if: ${{ always() && steps.rust_changes.outputs.changed ==
'true' }} with if: always() && steps.rust_changes.outputs.changed == 'true');
update the if for the step named "Upload coverage to Codecov" so it matches the
surrounding steps' style while keeping the same logical condition.
- Around line 26-47: The Rust-change detector in the rust_changes job can
produce false negatives because actions/checkout@v4's shallow clone plus the
later git fetch --no-tags --depth=1 origin "${{ github.base_ref }}" may produce
an incorrect/partial merge-base and an incomplete git diff; update the job to
fetch the full base history and avoid inline interpolation: change the checkout
to perform a full fetch (set fetch-depth: 0) or remove the --depth=1 from the
git fetch so the merge-base/diff are accurate, and pass github.base_ref into the
run step via an environment variable (use that env var instead of interpolating
"${{ github.base_ref }}") so the git commands (git merge-base, git diff
--name-only) reliably detect Rust changes.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 113-116: The benchmarks job is missing the Rust-change gating
because it can't read steps.rust_changes.outputs from the separate rust job;
modify the rust job to expose a job output (e.g. add outputs: rust_changes: ${{
steps.rust_changes.outputs.rust_changes }}) and then gate the benchmarks job by
referencing that job output (add needs: rust and if:
needs.rust.outputs.rust_changes == 'true' on the benchmarks job). Update the
benchmarks job to keep its runs-on/fetch-depth but only run when the exported
rust_changes job output indicates Rust files changed.
2 participants
🚀 OSCG'26 Contributor Notice
NexumDB participates in the Open Source Contributor Games 2026! High-quality PRs earn you points, recognition, and networking opportunities. Please follow our contribution guidelines for maximum impact and ensure your submission meets OSCG quality standards.
Summary
Replaced all user facing print statements with logging in the files nexum_ai/translator.py and nexum_ai/model_manager.py.
Closes #142
What Changed?
-Added module-level logger using logging.getLogger(name)
-Replaced print statements with appropriate logger levels (info, warning, error, exception)
Why?
Mixed stdout prints make CLI output noisy and harder to control compared to logger-based output.
Type of Change
Testing
Local Testing Checklist
cargo fmt --all -- --check- Code formattingcargo clippy --workspace --all-targets -- -D warnings- Lintingcargo test --workspace -- --test-threads=1- All tests passcargo audit- No security vulnerabilitiespython -m ruff check nexum_ai/- Python linting (if applicable)python -m compileall nexum_ai- Python compilation (if applicable)Manually executed translator functionality with sample natural language queries and confirmed generated SQL output remains unchanged from previous behavior.
Verified fallback translation path works correctly when LLM model is unavailable.
Tested model manager logic by checking existing model detection and simulated download scenarios to confirm correct logging messages appear for success, warning, and error cases.
Confirmed that no user-facing stdout print statements remain and output is now fully controlled via logging.
Verified logging integrates correctly with existing project logging configuration without overriding global settings.
Testing Details
Ran full pytest suite locally (97 tests).
Verified logging output appears correctly for success, warning, and error paths.
Ensured no behavioral changes compared to previous print-based output.
Screenshots/Examples
Performance Impact
Security Considerations
Documentation
demo.sh(if applicable)Pre-Merge Checklist
main🎯 OSCG'26 Quality Standards
By submitting this PR, I confirm:
Additional Notes
Followed existing project logging patterns and avoided adding global logging configuration assuming project-level setup exists.
Reviewers: Please check that all checkboxes are ticked before approving
Summary by CodeRabbit