feat: S08.03 slice 4 set command for in-RAM configuration injection#303
Conversation
Slice 3b.2 hardcoded the FreeRTOS example's identity, message body, and endpoint into Example/FreeRtos/SingleTask/main.c because the FreeRTOS port deliberately skips Example/Common/ExampleCommandLine.c (no getopt). Slice 4 makes those configurable in-RAM via a `set NAME VALUE` command parallel to `send` and `quit` on the existing UART grammar. Library-side dispatcher (host-TDD'd, 9 ZOMBIES cycles in Tests/Example/ExampleInteractiveTest.cpp): ExampleInteractive_Run gains an ExampleInteractiveSetHandler arg with shape `bool (*)(const char* name, const char* value)`. The runner splits args at first whitespace, calls the handler, echoes `set name=value` on true or prints `set: invalid` on false. NULL handler silently ignores `set`, mirroring `switch`. POSIX/Threaded/Windows callers pass NULL. FreeRTOS-side OnSet (integrator glue): mutable static arrays sized to RFC 5424 maxima, a file-static g_message holding facility/severity in place plus messageId/msg pointers into the storage, and g_port + g_endpointVersion behind GetEndpoint/GetEndpointVersion. Validation: identity / msgid / msg / host non-empty within storage cap; facility 0-23; severity 0-7; port 1-65535. Helpers TryUpdateString and TryParseUInt do the bounds-check + memcpy + null-terminate and the full-string strtol parse respectively. `host` is plumbed through GetEndpoint and accepted into g_host but SolidSyslogFreeRtosStaticResolver still routes via the hardcoded IPv4 octets — `set host 1.2.3.4` echoes confirmation but doesn't change the destination IP yet. Follow-up slice teaches the resolver to parse dotted-quads. QEMU smoke verifies: default hostname on port 5514, `set hostname Foo` mutates frames on next send, `set port 5515` routes to the new port (endpoint-version bump triggers UdpSender re-pull), and four `set: invalid` rejections (out-of-range, empty, unknown field). Closes #302 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds a ChangesSet Command for In-RAM Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1105 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Example/FreeRtos/SingleTask/main.c (1)
285-299: ⚡ Quick winPrefer
strtouloverstrtolfor parsing unsigned values.The function is designed to parse unsigned integers but uses
strtol(which parses signedlong). While negative values are correctly rejected (line 293), usingstrtoulwould be more semantically correct and would properly handle the full unsigned range and overflow conditions viaerrno.♻️ Proposed refactor to use strtoul
static bool TryParseUInt(const char* value, unsigned long* out) { if (*value == '\0') { return false; } - char* end = NULL; - long parsed = strtol(value, &end, 10); - if ((*end != '\0') || (parsed < 0)) + char* end = NULL; + unsigned long parsed = strtoul(value, &end, 10); + if (*end != '\0') { return false; } *out = (unsigned long) parsed; return true; }🤖 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 `@Example/FreeRtos/SingleTask/main.c` around lines 285 - 299, The TryParseUInt function currently uses strtol for unsigned parsing; replace its use with strtoul (and include errno.h) so it correctly parses the full unsigned range and detects overflow via errno==ERANGE; preserve the initial empty-string check, call strtoul(value, &end, 10), verify end points to '\0', check errno for ERANGE and treat that as failure, and reject values where end==value (no digits); on success assign the parsed unsigned long to *out.
🤖 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 `@DEVLOG.md`:
- Line 98: Update the unlabeled fenced code block for the QEMU smoke output so
markdownlint MD040 is satisfied by adding a language label (e.g., change the
opening ``` to ```text) for the block that starts with "PORT=5514 BYTES=<134>…
FreeRtosExample … (default)" and ends with "quit (clean exit)"; ensure the
opening fence uses the language token (text) while keeping the existing block
content unchanged.
In `@Example/Common/ExampleInteractive.h`:
- Around line 14-17: The new set callback lacks a void* context parameter;
update the ExampleInteractiveSetHandler typedef to accept a void* context as the
first (or last, consistent with ExampleInteractiveSwitchHandler) parameter and
add a matching void* context argument to the ExampleInteractive_Run signature so
callers can pass instance state; ensure ExampleInteractive_Run forwards that
context when invoking onSet (and keep the existing
ExampleInteractiveSwitchHandler/its usage consistent with the shared context
convention).
In `@Example/FreeRtos/SingleTask/main.c`:
- Around line 235-237: The host-handling branch currently calls
TryUpdateString(g_host, sizeof(g_host), value) but does not increment
g_endpointVersion; update the branch handling strcmp(name, "host") to increment
g_endpointVersion when TryUpdateString returns true (same pattern used for the
"port" branch), and optionally add a short comment referencing
GetEndpointVersion to indicate the version bump is intentional for host changes;
ensure you reference g_host, TryUpdateString, and g_endpointVersion in the
change.
---
Nitpick comments:
In `@Example/FreeRtos/SingleTask/main.c`:
- Around line 285-299: The TryParseUInt function currently uses strtol for
unsigned parsing; replace its use with strtoul (and include errno.h) so it
correctly parses the full unsigned range and detects overflow via errno==ERANGE;
preserve the initial empty-string check, call strtoul(value, &end, 10), verify
end points to '\0', check errno for ERANGE and treat that as failure, and reject
values where end==value (no digits); on success assign the parsed unsigned long
to *out.
🪄 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
Run ID: 23bcd7a6-b233-4bc3-a28d-b8c255ebf720
📒 Files selected for processing (9)
DEVLOG.mdExample/Common/ExampleInteractive.cExample/Common/ExampleInteractive.hExample/FreeRtos/SingleTask/main.cExample/SingleTask/SolidSyslogExample.cExample/Threaded/main.cExample/Windows/SolidSyslogWindowsExample.cTests/Example/CMakeLists.txtTests/Example/ExampleInteractiveTest.cpp
| typedef bool (*ExampleInteractiveSetHandler)(const char* name, const char* value); | ||
|
|
||
| void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch); | ||
| void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch, | ||
| ExampleInteractiveSetHandler onSet); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Add a context parameter to the new set callback API.
The new callback currently cannot carry per-instance state without globals. Please align this new callback family with the repository callback contract by adding a void* context parameter and a paired context argument in ExampleInteractive_Run.
Proposed API shape
-typedef bool (*ExampleInteractiveSetHandler)(const char* name, const char* value);
+typedef bool (*ExampleInteractiveSetHandler)(void* context, const char* name, const char* value);
-void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch,
- ExampleInteractiveSetHandler onSet);
+void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch,
+ ExampleInteractiveSetHandler onSet, void* context);As per coding guidelines: “For new callbacks, the function pointer must take a void* context parameter; config struct exposes paired context field; when several callbacks form a logical feature, share one context field.”
📝 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.
| typedef bool (*ExampleInteractiveSetHandler)(const char* name, const char* value); | |
| void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch); | |
| void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch, | |
| ExampleInteractiveSetHandler onSet); | |
| typedef bool (*ExampleInteractiveSetHandler)(void* context, const char* name, const char* value); | |
| void ExampleInteractive_Run(const struct SolidSyslogMessage* message, FILE* input, ExampleInteractiveSwitchHandler onSwitch, | |
| ExampleInteractiveSetHandler onSet, void* context); |
🤖 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 `@Example/Common/ExampleInteractive.h` around lines 14 - 17, The new set
callback lacks a void* context parameter; update the
ExampleInteractiveSetHandler typedef to accept a void* context as the first (or
last, consistent with ExampleInteractiveSwitchHandler) parameter and add a
matching void* context argument to the ExampleInteractive_Run signature so
callers can pass instance state; ensure ExampleInteractive_Run forwards that
context when invoking onSet (and keep the existing
ExampleInteractiveSwitchHandler/its usage consistent with the shared context
convention).
There was a problem hiding this comment.
Declined for this slice — leaving as-is.
Rationale: ExampleInteractive lives in Example/Common/, which is Tier 3 (best-effort examples, lowest support bar — see CLAUDE.md "Support tiers"). The Callback Conventions section is in scope for library callbacks (Tier 1/2 Core/, Platform/); the existing ExampleInteractiveSwitchHandler next to this one was added without context for the same reason. Adding context to Set now would also pull Switch into scope per the same section's "the class that owns them is next touched" rule (since this PR touches ExampleInteractive), widening the slice beyond config-injection.
The current FreeRTOS OnSet handler uses file-static globals, which is the documented pattern for the example tier — it's already what the existing POSIX ExampleAppName_Set / ExampleSwitchConfig_SetByName helpers do. There's no current consumer for instance-state, and CLAUDE.md "Don't add features beyond what the task requires" / "Don't design for hypothetical future requirements" applies.
If a multi-instance integrator surfaces a real need, we'll migrate Switch + Set together in one principled change rather than bolting context onto one of them now.
There was a problem hiding this comment.
@DavidCozens, understood — the Tier 3 / YAGNI reasoning is clear and well-documented. I'll withdraw the suggestion and record the exemption for future reviews.
✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 303
File: Example/Common/ExampleInteractive.h:14-17
Timestamp: 2026-05-09T15:56:13.853Z
Learning: In solid-syslog, the CLAUDE.md Callback Conventions section (void* context parameter + paired config field) applies only to Tier 1/2 library callbacks in Core/ and Platform/. Callbacks in Tier 3 example code (Example/Common/, Example/FreeRtos/, etc.) — such as ExampleInteractiveSwitchHandler and ExampleInteractiveSetHandler — are exempt from this requirement. The documented pattern for example-tier handlers is file-static globals. If a multi-instance use-case ever arises, Switch and Set should be migrated together in one principled change rather than piecemeal.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 283
File: Platform/OpenSsl/Interface/SolidSyslogTlsStream.h:7-7
Timestamp: 2026-05-07T08:22:53.110Z
Learning: In solid-syslog, the CLAUDE.md callback rule that requires a `void* context` parameter (and the paired config field) should be applied only to callbacks that need to carry real per-instance state. Exempt callbacks that are stateless thin wrappers over OS primitives—i.e., the callback is effectively a pure function of its input parameters with no instance state to preserve—so adding `void* context` would be pointless (the context would always be NULL). For example, platform implementations like `SolidSyslogSleepFunction` that wrap `nanosleep`/`Sleep` and are defined purely in terms of `int milliseconds` should not add `void* context` or the paired config field. For callbacks that do depend on per-instance data, include `void* context` and the paired config field as required by CLAUDE.md.
| if (strcmp(name, "host") == 0) | ||
| { | ||
| return TryUpdateString(g_host, sizeof(g_host), value); |
There was a problem hiding this comment.
Consider bumping g_endpointVersion on successful host changes for forward compatibility.
The PR objectives state "bump GetEndpointVersion on successful port (or host) changes", but this implementation only increments the version for port (line 247), not for host. While the current resolver ignores the host field (per the comment at lines 199–203), bumping the version on host changes would align with the stated objectives and ensure the infrastructure is in place when the resolver is updated in a follow-up slice to actually parse dotted-quads.
If deferring the version bump is intentional (to avoid unnecessary re-resolution while the resolver doesn't use host), consider documenting that decision in a comment or updating the objectives to clarify.
Proposed fix to bump version on host changes
if (strcmp(name, "host") == 0)
{
- return TryUpdateString(g_host, sizeof(g_host), value);
+ if (TryUpdateString(g_host, sizeof(g_host), value))
+ {
+ g_endpointVersion++;
+ return true;
+ }
+ return false;
}📝 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.
| if (strcmp(name, "host") == 0) | |
| { | |
| return TryUpdateString(g_host, sizeof(g_host), value); | |
| if (strcmp(name, "host") == 0) | |
| { | |
| if (TryUpdateString(g_host, sizeof(g_host), value)) | |
| { | |
| g_endpointVersion++; | |
| return true; | |
| } | |
| return false; | |
| } |
🤖 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 `@Example/FreeRtos/SingleTask/main.c` around lines 235 - 237, The host-handling
branch currently calls TryUpdateString(g_host, sizeof(g_host), value) but does
not increment g_endpointVersion; update the branch handling strcmp(name, "host")
to increment g_endpointVersion when TryUpdateString returns true (same pattern
used for the "port" branch), and optionally add a short comment referencing
GetEndpointVersion to indicate the version bump is intentional for host changes;
ensure you reference g_host, TryUpdateString, and g_endpointVersion in the
change.
There was a problem hiding this comment.
Declined — leaving as-is.
Bumping g_endpointVersion on a successful host mutation today would cause SolidSyslogUdpSender to re-pull the endpoint and the resolver to re-resolve, but SolidSyslogFreeRtosStaticResolver still ignores endpoint->host and routes via the Create-time IPv4 octets (see comment in GetEndpoint). That's wasted work without any behavioral change.
The bump lands naturally in the follow-up slice that teaches FreeRtosStaticResolver to parse dotted-quads from g_host (see ## Out of scope in #302 — "SolidSyslogFreeRtosStaticResolver dotted-quad parsing — separate follow-up so set host actually changes the destination IP"). The DEVLOG entry phrases this as port (or eventually host) — emphasis on eventually — so the deferral is documented.
Aligns with CLAUDE.md "Don't design for hypothetical future requirements".
There was a problem hiding this comment.
@DavidCozens, understood — the reasoning is sound. Triggering re-resolution while the resolver ignores endpoint->host is pure overhead, and CLAUDE.md's stance against designing for hypothetical future requirements means the bump belongs in the slice that actually wires up dotted-quad parsing. I'll drop this concern.
✏️ Learnings added
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 303
File: Example/FreeRtos/SingleTask/main.c:235-237
Timestamp: 2026-05-09T15:56:18.170Z
Learning: In DavidCozens/solid-syslog, `g_endpointVersion` is intentionally NOT bumped when `set host` succeeds (in `Example/FreeRtos/SingleTask/main.c`, `OnSet` handler). `SolidSyslogFreeRtosStaticResolver` currently ignores `endpoint->host` and routes via hardcoded `Create`-time IPv4 octets, so bumping the version would cause wasted re-resolution with no behavioral change. The version bump for host changes is deferred to the follow-up slice that teaches the resolver to parse dotted-quads (tracked as out-of-scope in issue `#302`). Do not raise a review finding for the missing version bump on host changes until that follow-up slice lands.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 153
File: Example/Windows/SolidSyslogWindowsExample.c:45-55
Timestamp: 2026-04-18T07:18:35.201Z
Learning: In this repository’s example entry-point C files under `Example/` (e.g., `SolidSyslogWindowsExample.c`, `SolidSyslogExample.c`), it is intentional to omit `argc/argv` NULL/length guards before using `argv[0]` (e.g., calling `ExampleAppName_Set(argv[0])`). C guarantees that when invoked via `main`, `argc > 0` and `argv[0]` is a valid string (§5.1.2.2.1), and the project policy (CLAUDE.md) forbids defensive checks for impossible scenarios. Therefore, do not raise review findings for missing `argc/argv` guards in these example entry points.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 153
File: Example/Windows/SolidSyslogWindowsExample.c:57-66
Timestamp: 2026-04-18T07:18:55.197Z
Learning: In this project’s example entry-point C files, do not flag/require NULL checks after calls to the solid-syslog `Create` functions (e.g., `SolidSyslogWinsockResolver_Create`, `SolidSyslogWinsockDatagram_Create`, `SolidSyslogUdpSender_Create`, `SolidSyslogNullBuffer_Create`, `SolidSyslogNullStore_Create`, `SolidSyslogAtomicCounter_Create`, `SolidSyslogMetaSd_Create`, `SolidSyslogTimeQualitySd_Create`, `SolidSyslogOriginSd_Create`). These `Create` APIs return pointers to static instances and are designed not to fail at construction time; any runtime failures that can occur are surfaced later via the vtable at use-time (e.g., host resolution). Therefore, NULL checks after `Create` calls in example files (such as `Example/Windows/SolidSyslogWindowsExample.c` and similar examples) are intentionally omitted and should not be treated as a bug.
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 291
File: Example/FreeRtos/HelloWorld/CmsdkUart.c:17-42
Timestamp: 2026-05-08T17:19:38.382Z
Learning: In this codebase (DavidCozens/solid-syslog), internal driver/config/access structs may intentionally store raw pointers without NULL checks, `isInitialized` flags, or defensive struct copies. Treat this as an explicit project policy: do not flag store-by-pointer usage or missing NULL/initialization guards for these internal config/access structs when the pointer is provided by production callers and is guaranteed to point to `static const` objects with program lifetime (per CLAUDE.md). Only flag NULL/initialization issues when the pointer originates from potentially invalid/external/untrusted sources or when the code does not establish lifetime/guarantees.
Review summary — fixes incoming + decisionsAccepted (applied in next commit):
Also fixed (CI surfaced these alongside the CodeRabbit findings):
Declined (rationale on each thread):
Local verification before pushing the fix-up commit: |
cppcheck (`[cstyleCast]`): replace `(void*) input` in ExampleInteractiveTest's RunWithInput with `const_cast<char*>(input)` + rationale comment (fmemopen mode "r" never writes to the buffer). clang-tidy (analyze-tidy): - SetHandlerSpy: in-class default initializers for callCount and returnValue (cppcoreguidelines-pro-type-member-init). - Stdout-capture helpers + RunWithInput: NOLINT comments for the owning-memory checks on dup / freopen / fopen / fclose ×2 and the clang-analyzer-unix.Stream check on the fread loop, matching the pattern already established in SolidSyslogExampleTest.cpp for the same POSIX-C test-helper idioms. - const_cast in RunWithInput: NOLINT for cppcoreguidelines-pro-type-const-cast cross-referencing the existing rationale comment. CodeRabbit nitpick (TryParseUInt): switch from `strtol` + explicit `< 0` rejection to `strtoul` for semantic correctness on unsigned parsing. Behaviour preserved because each call site already range-checks (port <= UINT16_MAX, facility <= 23, severity <= 7) and strtoul's wrap-on-negative is caught by those upper bounds; comment documents the contract. CodeRabbit MD040: add `text` language tag to the QEMU smoke fenced block in the DEVLOG slice-4 entry. Declined (replies on threads, no code change): - ExampleInteractiveSetHandler context parameter — Tier 3 example code; would also pull Switch migration into scope per CLAUDE.md "the class that owns them is next touched". - g_endpointVersion bump on host mutation — wasted work today (resolver still ignores endpoint->host); lands in the follow-up slice that wires the resolver to dotted-quad parsing. Local verification: cppcheck preset clean, tidy preset clean, freertos-cross builds clean, ExampleTests 63/63 (77 checks), clang-format --dry-run --Werror clean on all touched files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1105 passed, 🙈 3 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Summary
Example/Common/ExampleInteractivewith aset NAME VALUEdispatcher parallel toswitchandquit— host-TDD'd in 9 ZOMBIES cycles. Library splits args; an integrator-suppliedbool (*)(const char* name, const char* value)handler decides accept/reject. Runner echoesset name=valueontrue, printsset: invalidonfalse, silently ignoressetlines when handler isNULL(mirrorsswitch).OnSetinExample/FreeRtos/SingleTask/main.cmutates static backing for v1 fields (hostname, appname, procid, msgid, msg, host, port, facility, severity) with RFC 5424 length / range validation.g_messageholds facility/severity + pointers into the mutable storage;g_portandg_endpointVersionback the endpoint callbacks (the version bump onset portis what makes UdpSender re-pull and route to the new port).hostis plumbed throughGetEndpointbutSolidSyslogFreeRtosStaticResolverstill uses the hardcoded IPv4 octets —set host …echoes confirmation but doesn't change the destination IP. Follow-up slice teaches the resolver to parse dotted-quads.Closes #302.
Test plan
clang-format --dry-run --Werroron every changed file — clean.ctest --preset debug— 5/5 (ExampleTests63/63 with 9 newExampleInteractivetests).cmake --build --preset freertos-cross— both ELFs link clean.set hostname QemuFoomutates next frame,set port 5515routes to new port, fourset: invalidrejections (facility=99, port=99999, empty hostname value, unknown field), cleanquit.analyze-tidy,analyze-cppcheck,analyze-iwyu,analyze-format,build-windows-msvc, BDD, OpenSSL integration, coverage.🤖 Generated with Claude Code
Summary by CodeRabbit