No actionable comments were generated in the recent review. 🎉

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d4f04b8d-e51d-4b9e-9687-75343d112f5a

Reviewing files that changed from the base of the PR and between ac68fce and 71fe7d6.

• json_logging/util.py
• tests/test_fastapi.py

Walkthrough

The request call-stack inspection now iterates over a snapshot of frame locals. A FastAPI regression test verifies safe behavior when tracing mutates those locals.

Changes

Request stack safety

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

Mergeability Score: ⚪ Minimal · up to 71fe7

This localized change prevents request lookup failures when frame locals are modified during tracing, with regression coverage and reported checks passing; no actionable merge-blocking risk remains.

• Create PR with unit tests

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.

• X
• Mastodon
• Reddit
• LinkedIn

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

PR Summary by Qodo

Snapshot frame locals during request lookup to avoid mutation errors

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

• Snapshot frame.f_locals.items() before scanning to avoid dict-resize during iteration.
• Reuse snapshotted values during request-type checks for consistent inspection.
• Add FastAPI regression test that mutates frame.f_locals via sys.settrace().

graph TD
  A["RequestUtil.get_request_from_call_stack()"] --> B{"Iterate frame locals"} --> C["Snapshot: tuple(f_locals.items())"] --> D["Type-check values"] --> E["Return request or None"]
  T["Trace hook (sys.settrace)"] --> F["Mutates frame.f_locals"]
  F --> C
  subgraph Legend
    direction LR
    _fn["Function"] ~~~ _dec{"Decision"} ~~~ _test["Test/Hook"]
  end

Loading

The following are alternative approaches to this PR:

1. Iterate over list(f_locals) and re-index values

• ➕ Avoids 'dict changed size' by snapshotting keys only
• ➕ Slightly less memory than snapshotting (key, value) pairs
• ➖ Still reads values from the live dict, so values could change between key snapshot and lookup
• ➖ Extra dict lookup per key

2. Copy locals dict via f_locals.copy()

• ➕ Clear intent: stable snapshot of locals
• ➕ Provides both keys and values consistently
• ➖ Potentially higher overhead than tuple(items()) (creates a full dict)
• ➖ May differ in subtle ways if code relies on dict identity/ordering (unlikely here)

Recommendation: Keep the current approach (tuple(f_locals.items())) because it provides a stable, minimal snapshot (including values) and avoids additional live-dict reads during type checks. The added regression test using sys.settrace() is a strong guard against reintroducing the mutation-during-iteration failure mode.

Bug fix (1) +7 / -4

util.pySnapshot frame locals items to prevent mutation during stack inspection +7/-4

Snapshot frame locals items to prevent mutation during stack inspection

• Changes frame-local scanning to iterate over 'tuple(f_locals.items())' and reuse the snapshotted value when checking request types, preventing failures when tracing/debug hooks mutate 'f_locals' mid-iteration. Also normalizes the file to include a trailing newline.

json_logging/util.py

Tests (1) +32 / -0

test_fastapi.pyAdd regression test for frame locals mutation during request lookup +32/-0

Add regression test for frame locals mutation during request lookup

• Adds a FastAPI-focused regression test that installs a trace function via 'sys.settrace()' and mutates 'frame.f_locals' after iteration begins inside 'get_request_from_call_stack()'. Verifies the lookup completes without error and that the mutation path was exercised.

tests/test_fastapi.py

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Description

get_request_from_call_stack() now allocates tuple(f_locals.items()) for every inspected frame,
creating an O(n) tuple of 2-tuples per frame walked. This increases CPU/memory overhead for
correlation-id lookup during log formatting in FastAPI, where the library falls back to stack
scanning (no global request object).

Code

json_logging/util.py[190]

+            for key, value in tuple(f_locals.items()):

Evidence

The PR introduces a per-frame tuple(f_locals.items()) allocation during stack scanning. This scan
is triggered during log formatting when correlation-id is missing, and in FastAPI the library
explicitly cannot rely on a global request object, making stack scanning more common.

json_logging/util.py[180-200]
json_logging/formatters.py[141-154]
json_logging/util.py[124-152]
json_logging/framework/fastapi/implementation.py[61-69]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RequestUtil.get_request_from_call_stack()` snapshots `tuple(f_locals.items())`, which allocates a tuple plus a 2-tuple per local entry for every frame visited.

## Issue Context
This codepath is exercised from `JSONLogWebFormatter` via `request_util.get_correlation_id(within_formatter=True)`. For FastAPI, `support_global_request_object()` is `False`, so correlation-id lookup may frequently fall back to scanning the call stack.

## Fix Focus Areas
- json_logging/util.py[180-200]

### Suggested approach
Preserve mutation-safety but reduce allocations by snapshotting only values:
- Keep the existing fast-path checks for `request` and `req`.
- Replace `for key, value in tuple(f_locals.items()): ...` with `for value in tuple(f_locals.values()): ...` (and drop the key filter), since you only need to detect any local whose value is an instance of `class_type`.
- Ensure the regression test still passes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗