Skip to content

fix: log swallowed lazy-import failures to aid diagnosability#2899

Open
praisonai-triage-agent[bot] wants to merge 1 commit into
mainfrom
claude/issue-2897-20260710-1714
Open

fix: log swallowed lazy-import failures to aid diagnosability#2899
praisonai-triage-agent[bot] wants to merge 1 commit into
mainfrom
claude/issue-2897-20260710-1714

Conversation

@praisonai-triage-agent

@praisonai-triage-agent praisonai-triage-agent Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #2897

Summary

The package-level lazy loader (create_lazy_getattr_with_fallback in praisonaiagents/_lazy.py) swallowed ImportError/AttributeError for any mapped public symbol, returned None, and cached it for the process lifetime. This masked real import bugs and stale _LAZY_IMPORTS mapping entries as confusing TypeError: 'NoneType' object is not callable errors far from the root cause.

Change (purely additive)

  • Preserve the existing graceful None fallback and its caching β€” no behaviour/API change.
  • Log the swallowed exception (name, module path, attr, original error) so failures are diagnosable.
  • Distinguish the two failure modes:
    • ImportError (genuinely-absent optional module) β†’ DEBUG (stays quiet by default).
    • AttributeError (module imports but attribute missing β†’ stale/renamed mapping) β†’ WARNING (louder signal).
  • Logging only fires on the error branch, so there is no import-time or hot-path cost.

Validation

  • Added ad-hoc test confirming: missing module β†’ None + DEBUG log; stale attribute β†’ None + WARNING log; valid symbol resolves; cached None preserved.
  • Verified import praisonaiagents still works cleanly.

Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of unavailable or renamed lazy-loaded attributes.
    • Prevented lookup failures from interrupting application behavior by safely returning an unavailable value.
  • Diagnostics

    • Added more specific logging for missing attributes and failed imports, making troubleshooting clearer.

…2897)

The package-level lazy loader silently converted real ImportError/
AttributeError into a cached None, masking genuine import bugs and stale
mapping entries as confusing NoneType errors far from the root cause.

This additive change preserves the graceful None fallback but logs the
swallowed exception: DEBUG for a genuinely-absent optional module and
WARNING for a missing attribute (which usually signals a stale mapping).

Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
@MervinPraison

Copy link
Copy Markdown
Owner

@coderabbitai review

@MervinPraison

Copy link
Copy Markdown
Owner

/review

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more β†’

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account β†’

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us β†’

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor
βœ… Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@MervinPraison MervinPraison added pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:manual-review Blocked: requires manual review pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet pipeline/final-claude-pending Reviews done; waiting for FINAL @claude labels Jul 10, 2026
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. πŸŽ‰

ℹ️ Recent review info
βš™οΈ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6d5e70e5-015a-460c-873e-938703a0cc74

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between ca6489a and 50097e5.

πŸ“’ Files selected for processing (1)
  • src/praisonai-agents/praisonaiagents/_lazy.py

πŸ“ Walkthrough

Walkthrough

The lazy loader now initializes a module logger and distinguishes AttributeError from ImportError, logging each failure type while retaining the existing cached None fallback.

Changes

Lazy loader diagnostics

Layer / File(s) Summary
Lazy import failure logging
src/praisonai-agents/praisonaiagents/_lazy.py
Adds module-level logging, warns about potentially stale attribute mappings, and debug-logs import failures while preserving cached None results.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: MervinPraison

πŸš₯ Pre-merge checks | βœ… 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
βœ… Passed checks (4 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title accurately summarizes the main change: adding logs for swallowed lazy-import failures.
Linked Issues check βœ… Passed The PR preserves cached None fallback and adds distinct logging for ImportError and AttributeError as requested by #2897.
Out of Scope Changes check βœ… Passed The changes stay focused on _lazy.py and do not introduce unrelated modifications.
✨ Finishing Touches
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-2897-20260710-1714

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.

❀️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds diagnostics for swallowed lazy-import failures. The main changes are:

  • Adds a module logger in praisonaiagents._lazy.
  • Logs missing lazy-import attributes as warnings.
  • Logs missing optional lazy-import modules as debug messages.
  • Keeps the existing cached None fallback behavior.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The logging runs after the lazy import failure is caught and keeps the previous None fallback.
  • The added logger uses standard library logging and does not change the import cache contract.

Important Files Changed

Filename Overview
src/praisonai-agents/praisonaiagents/_lazy.py Adds logging to the lazy-loader fallback branches without changing the existing return or cache behavior.

Reviews (1): Last reviewed commit: "fix: log swallowed lazy-import failures ..." | Re-trigger Greptile

@MervinPraison

Copy link
Copy Markdown
Owner

@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding.

Phase 1: Review per AGENTS.md

  1. Protocol-driven: check heavy implementations vs core SDK
  2. Backward compatible: ensure zero feature regressions
  3. Performance: no hot-path regressions
  4. SDK value: review in depth whether the change genuinely adds value to the SDK β€” never add features for the sake of adding them. It must strengthen the SDK (simpler, more user-friendly, robust, world-class, secure). If it does not clearly add value, request changes or recommend rejecting/closing rather than merging scope creep
  5. Do not bloat the Agent class with additional params β€” only if absolutely required; we already support many params.
  6. Repo routing: agent-callable tools β†’ PraisonAI-Tools; lifecycle plugins β†’ PraisonAI-Plugins; optional sandbox backends β†’ PraisonAI-Plugins (praisonai.sandbox entry point) β€” request changes if wrongly added to praisonaiagents/

Phase 2: FIX Valid Issues
7. For any VALID bugs or architectural flaws found by Gemini, CodeRabbit, Qodo, Copilot, or any other reviewer: implement the fix
8. Push all code fixes directly to THIS branch (do NOT create a new PR)
9. Comment a summary of exact files modified and what you skipped

Phase 3: Final Verdict
10. If all issues are resolved, approve the PR / close the Issue
11. If blocking issues remain, request changes / leave clear action items

@MervinPraison MervinPraison added pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:cooldown Blocked: post-push or @claude cooldown and removed pipeline/final-claude-pending Reviews done; waiting for FINAL @claude pipeline/blocked:no-final Blocked: no FINAL @claude trigger yet labels Jul 10, 2026
@praisonai-triage-agent

praisonai-triage-agent Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Claude finished @MervinPraison's task β€”β€” View job


Final Architecture Review β€” PR #2899 βœ… Approve

  • Read AGENTS.md guidelines
  • Review changed file _lazy.py
  • Validate architecture (routing, backward-compat, performance, SDK value)
  • Check reviewer feedback for valid issues
  • Fix any valid issues + push
  • Final verdict

Scope: Single file src/praisonai-agents/praisonaiagents/_lazy.py (+23/-2). Python-only, in-scope. No praisonai-rust/praisonai-ts touched.

Phase 1 β€” AGENTS.md checks

Check Result
Protocol-driven / core placement βœ… Diagnostics logging in an internal helper β€” correct layer, not a heavy impl
Backward compatible βœ… Graceful None fallback + caching preserved; no API/behaviour change
Performance (hot path) βœ… Logging fires only on the exception branch; zero cost on cache-hit / success path
SDK value βœ… Real value: converts confusing NoneType is not callable failures into diagnosable logs; sensibly splits ImportErrorβ†’DEBUG (quiet) vs AttributeErrorβ†’WARNING (loud, stale-mapping signal)
Agent class bloat βœ… N/A β€” no Agent params added
Repo routing βœ… Correct repo; not a tool/plugin/sandbox concern

Phase 2 β€” Reviewer feedback

  • Greptile: 5/5, safe to merge β€” no blocking issues.
  • CodeRabbit: processing; walkthrough only, no actionable findings.
  • Qodo: reviews paused (account/plan), no findings.

No valid bugs or architectural flaws raised. One minor note (non-blocking): the lazy_import docstring mentions ModuleNotFoundError; since it's a subclass of ImportError, the new except ImportError correctly covers it β€” behaviour is sound.

Validation (run locally)

  • import praisonaiagents β†’ OK (clean).
  • Missing module β†’ None + DEBUG log βœ…
  • Stale attribute β†’ None + WARNING log βœ… (verified emitted)
  • Valid symbol β†’ resolves correctly βœ…
  • Exception ordering (AttributeError before ImportError) is correct; neither subclasses the other.

Phase 3 β€” Verdict

Approve. Purely additive, backward-compatible diagnostics that strengthen the SDK's debuggability with no hot-path cost. No changes needed β€” nothing pushed.
Β· Branch: claude/issue-2897-20260710-1714

@praisonai-triage-agent praisonai-triage-agent Bot removed pipeline/blocked:ci Blocked: CI not green on HEAD pipeline/blocked:cooldown Blocked: post-push or @claude cooldown labels Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pipeline/awaiting-merge-gate FINAL done; waiting for merge gate / CI pipeline/blocked:manual-review Blocked: requires manual review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core SDK: lazy loader silently masks real import failures as None (and caches them permanently)

1 participant