feat(nodes): add Guild.ai tool node - #1689
Conversation
Add `tool_guild`, a dual data+tool node that runs Guild.ai agents from a RocketRide pipeline or an agent. Mirrors `tool_n8n` (dual-node structure, lane + tool faces, result mode) and `tool_v0` (IGlobal/test conventions, raise-not-return error handling). - Pipeline face: lane input runs the configured Guild agent; the answer is emitted on answers/text/documents/table. - Tool face: run_agent / get_session / get_session_events let an agent delegate to and follow up on Guild sessions. - HTTP Basic auth with a Guild trigger API key; per-run session budget; POSTs are never retried (each session is billed); agent-supplied identifiers are sanitised to plain path segments. Tests: 82 unit + contract tests; verified end-to-end against a live Guild workspace (api_trigger session over Basic auth; session/status/event response shapes confirmed and pinned as regression fixtures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
|
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:
📝 WalkthroughWalkthroughAdds a Guild.ai integration with shared configuration, authenticated session APIs, tool and pipeline execution modes, service metadata, documentation, example pipelines, dependency setup, and network-free tests. ChangesGuild.ai node integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Pipeline
participant IInstance
participant guild_client
participant GuildAPI
Pipeline->>IInstance: Submit pipeline input
IInstance->>guild_client: Start Guild session
guild_client->>GuildAPI: POST session request
GuildAPI-->>guild_client: Return session identifier
IInstance->>guild_client: Poll and fetch session events
guild_client->>GuildAPI: GET status and transcript
GuildAPI-->>IInstance: Return status and events
IInstance-->>Pipeline: Emit text, answers, documents, and table outputs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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
🤖 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 `@examples/guild-delegate-agent.pipe`:
- Around line 27-32: Update the instruction string in the guild delegation
example to prohibit retrying any uncertain tool_guild_1.run_agent call,
including timeouts, transport failures, or surfaced structured errors, because
the billed non-idempotent session may already have started. Clarify that
failures are raised for the engine to surface rather than returned as
success:false, and direct follow-up to tool_guild_1.get_session or
tool_guild_1.get_session_events when session status is uncertain.
In `@nodes/src/nodes/tool_guild/guild_client.py`:
- Around line 163-167: Update the docstrings for the authenticated API-call
function at nodes/src/nodes/tool_guild/guild_client.py:163-167 and the
session/deadline handling function at
nodes/src/nodes/tool_guild/guild_client.py:305-310. Document ValueError only for
missing credentials in the first, RuntimeError for transport or HTTP failures
there, and change the second function’s session/deadline exception description
to RuntimeError; no implementation changes are needed.
- Around line 343-355: Replace the denylist validation in the
identifier-validation helper with an allowlist permitting only plain identifier
characters, while preserving required-value handling and the existing
invalid-field error behavior. Ensure valid hyphenated, underscored, dotted,
alphanumeric, and UUID-style values remain accepted, while encoded separators,
semicolon parameters, whitespace, control characters, and other characters are
rejected for agent, session_id, owner, and workspace inputs.
In `@nodes/src/nodes/tool_guild/IInstance.py`:
- Around line 187-193: Remove the self._require_workspace() call from the
get_session and corresponding events tool methods, since session-id endpoints
only require the existing session identifier. Keep _require_workspace() in the
session-start path unchanged.
- Line 252: Rename the open method parameter from object to obj in the IInstance
class, preserving its Entry type annotation and updating any references within
the method body to use obj.
- Around line 276-282: Update _build_input in IInstance so _text_parts and
_documents are joined with newline separators instead of an empty string,
preserving each content part byte-exactly while retaining boundaries between
adjacent chunks and documents. Update the expected input in
test_pipeline_closing_emits_answer_on_connected_lanes_only to reflect the
inserted newlines.
In `@nodes/src/nodes/tool_guild/README.md`:
- Around line 113-115: Update the README’s “Every field” statement to clarify
that environment fallbacks apply only to base URL, key ID/secret, owner,
workspace, and agent; do not imply env support for resultMode, timeout,
maxSessions, or verifyTls.
In `@nodes/test/test_tool_guild.py`:
- Around line 643-661: Update the _make_instance helper to accept the pytest
monkeypatch fixture and replace the direct _imod.guild_client assignment with
monkeypatch.setattr(_imod, 'guild_client', gclient), ensuring the module-level
client binding is restored after each test.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4deed526-ccc6-4e04-bff0-2376eba3ba81
⛔ Files ignored due to path filters (1)
nodes/src/nodes/tool_guild/guild.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
examples/guild-agent.pipeexamples/guild-delegate-agent.pipenodes/src/nodes/tool_guild/IGlobal.pynodes/src/nodes/tool_guild/IInstance.pynodes/src/nodes/tool_guild/README.mdnodes/src/nodes/tool_guild/__init__.pynodes/src/nodes/tool_guild/guild_client.pynodes/src/nodes/tool_guild/requirements.txtnodes/src/nodes/tool_guild/services.jsonnodes/test/test_tool_guild.py
- get_session / get_session_events no longer require workspace config; those endpoints are addressed by session id alone, so an agent can follow up with only the id from an earlier run_agent. - safe_segment now validates path-segment identifiers with an allowlist ([A-Za-z0-9._~-]) instead of a denylist, rejecting percent-encoding and other smuggling the denylist missed. - _build_input joins lane parts with a newline so adjacent chunks/documents keep a boundary instead of running together. - Correct guild_client docstrings (transport/HTTP failures raise RuntimeError, not ValueError) and the README env-fallback scope (connection fields only). - Strengthen the delegate example's no-retry guidance (a timed-out or errored run may already be a billed session) and rename the open() shadow of builtin. - Tests use monkeypatch to swap the stubbed guild_client so it can't leak. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The services.json invariant tests read the file with `read_text()` and no explicit encoding, which uses the locale default. In CI the bundled Python defaults to ASCII, so the em-dashes in the field descriptions raised UnicodeDecodeError and failed the four services tests (they passed locally only because the dev locale is UTF-8). Match the tool_n8n convention and decode as UTF-8 explicitly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Addressed the CodeRabbit review in Review items (all 8):
CI fix: the four |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@examples/guild-delegate-agent.pipe`:
- Around line 30-32: Update the retry guidance in the tool_guild_1 instructions
so wrong agent names or session IDs are retried only when the error conclusively
proves no session was created. Otherwise treat the outcome as uncertain: use
tool_guild_1.get_session and tool_guild_1.get_session_events to inspect the
existing session, or report the failure without starting another billed session.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: cb7f07e9-0cc2-43f7-b34e-8dbf8e2801ba
📒 Files selected for processing (5)
examples/guild-delegate-agent.pipenodes/src/nodes/tool_guild/IInstance.pynodes/src/nodes/tool_guild/README.mdnodes/src/nodes/tool_guild/guild_client.pynodes/test/test_tool_guild.py
Only retry a failed run_agent when the error conclusively proves no session was started; a wrong agent name does not by itself establish that Guild rejected the request before creating a billed session. Otherwise inspect the existing session or report the uncertain outcome. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
asclearuc
left a comment
asclearuc
left a comment
There was a problem hiding this comment.
Thanks @EdwardLien0426 — this is a careful piece of work. Isolating the whole wire contract in guild_client.py, claiming the session budget before the billable POST, and pinning regression fixtures from a real Guild session are all exactly right. The CodeRabbit points were properly fixed rather than waved away, and CI is fully green.
I am requesting changes for two correctness issues and one coverage gap. Details are in the inline comments.
Blocking
extract_outputcan emit internal noise as the agent's answer (guild_client.py:479). The final fallback takes the last event with any text, without checking its type. Running it against your own fixture shapes returns the user's own input, or the literal string'text', or an intermediate progress step — as the answer, with no error.get_session_eventsreads only the first page (guild_client.py:288). Your captured fixture showspagination: {has_more, limit, offset, total_count}and events arrive oldest-first, so the answer is always last. A transcript over 100 events loses the answer and then hits issue 1.- Missing tests.
beginGlobal,validateConfig, and the three public tool functions have no tests.
Smaller points, non-blocking
guild_client.py:49still says "UNVERIFIED AGAINST A LIVE WORKSPACE". That now contradicts the per-functionVERIFIED:notes and the PR description.IGlobal.py:95and:99re-implementconfig_intfromai.common.utils.services.jsontest.requires: the two credential env vars do not map to the node's config field names.- Prefer
from requests.status_codes import codes as status_codesover the raw numbers inguild_client.py. examples/README.mdhas a### <name>.pipesection per example. The two new.pipefiles are not listed there, so a user browsing that page will not find them. (I could not comment on this inline — that file is not part of the diff.)
The structure and the safety design are solid. Fix the two output-correctness paths and add the missing tests and this is ready.
Two output-correctness bugs plus the coverage gaps from @asclearuc's review. Blocking: - extract_output no longer emits internal noise as the answer. The blind "last event with any text" fallback could return the user's own echo, a span marker's literal 'text', or an intermediate progress step. It now skips a known-noise deny-list and returns '' when no real answer is present — an empty answer is a better failure than a wrong one. - get_session_events now follows pagination. Events arrive oldest-first so the answer is on the last page; the single-page read dropped it on any transcript past one page. Pages are followed via offset until has_more is false, bounded by MAX_EVENT_LIMIT. - Added tests for beginGlobal (env fallbacks, clamps, CONFIG-mode early return, result_mode), validateConfig (all warning branches), the three public tool functions, pagination, and the noise-vs-answer paths. The weak `assert out in (...)` that accepted the wrong answer now asserts one value. Non-blocking: - Rewrote the guild_client header (VERIFIED, not UNVERIFIED) and the stale two-segment endpoint path. - IGlobal uses config_int for timeout/maxSessions (a 0 now means "default", not "5 seconds"). - services.json test credentials now map to config fields: added KEY_ID / KEY_SECRET to the test framework's _ENV_ATTR_MAP (shared file — see below). - Use requests.status_codes names instead of magic HTTP numbers. - List the two guild example pipes in examples/README.md. Shared-code change: nodes/test/framework/pipeline.py gains two _ENV_ATTR_MAP entries (KEY_ID -> apiKeyId, KEY_SECRET -> apiKeySecret). No other node uses those env-var tokens, so the addition is purely additive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough review — the two output-correctness paths were the important ones. Addressed in Blocking
Non-blocking (all done)
Shared-code change (flagging explicitly): 100 unit + contract tests pass (up from 82), verified under a forced ASCII locale. |
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 (1)
nodes/src/nodes/tool_guild/IGlobal.py (1)
71-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing PEP 257 docstrings on
beginGlobalandvalidateConfig.Both are substantial, branchy methods (config loading with env fallbacks/clamping; five-branch config validation) yet neither has a docstring, unlike
claim_session_slotin the same file. As per path instructions,nodes/**/*.pyrequires "PEP 257 docstrings."📝 Proposed docstrings
def beginGlobal(self) -> None: + """Load Guild connection settings, credentials, and session-budget state from config.""" # Opened only to render the config UI — never touch the network or readdef validateConfig(self) -> None: + """Warn about canvas-visible misconfigurations without touching the network.""" # Config-only checks (no network) so this is safe to run during canvasAlso applies to: 121-156
🤖 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 `@nodes/src/nodes/tool_guild/IGlobal.py` around lines 71 - 106, Add PEP 257-compliant docstrings to the beginGlobal and validateConfig methods in IGlobal, briefly documenting their configuration initialization and validation responsibilities. Keep the existing branching, fallback, clamping, and validation behavior unchanged, matching the style of the existing claim_session_slot docstring.Source: Path instructions
🤖 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 `@examples/README.md`:
- Around line 149-151: Update the documentation section for guild-agent.pipe to
state that its Guild session has a finite timeout and that timed-out or errored
runs may still create a billed, non-idempotent session. Add the same “do not
retry blindly” guidance used in the guild-delegate-agent.pipe section.
---
Outside diff comments:
In `@nodes/src/nodes/tool_guild/IGlobal.py`:
- Around line 71-106: Add PEP 257-compliant docstrings to the beginGlobal and
validateConfig methods in IGlobal, briefly documenting their configuration
initialization and validation responsibilities. Keep the existing branching,
fallback, clamping, and validation behavior unchanged, matching the style of the
existing claim_session_slot docstring.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1fad069a-7f21-4bbb-9d3a-09cc737ca15b
📒 Files selected for processing (5)
examples/README.mdnodes/src/nodes/tool_guild/IGlobal.pynodes/src/nodes/tool_guild/guild_client.pynodes/test/framework/pipeline.pynodes/test/test_tool_guild.py
… step The pipeline face is deterministic (no agent retry loop), so the delegate example's "do not retry blindly" guidance does not map to it. Add the accurate equivalent: a run starts a billed Guild session, and a timeout does not cancel it on Guild's side, so re-running the pipeline starts a new one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
asclearuc
left a comment
asclearuc
left a comment
There was a problem hiding this comment.
Thanks @EdwardLien0426 — good, fast turnaround, and you fixed the right things rather than the easy ones. I re-verified each fix by running the code, not by reading the description.
Verdict: requesting changes
Why: the pagination fix changed the meaning of the limit parameter, and neither place that documents it was updated. limit used to cap how many events came back. It is now the page size, and the call returns the whole transcript regardless. The agent-facing tool description (IInstance.py:209) and the node README (README.md:85) both still say "Max events to return".
That is blocking for two reasons. First, an LLM picks its argument from that description, so the wrong text steers the agent to the worst value. Second, CLAUDE.md requires the co-located doc to change in the same commit as the contract it documents, and this commit changed the contract without touching either file.
Everything else below is non-blocking. This is a small, contained fix — not a rework.
The blocking issue in detail
Measured against a 1000-event transcript:
| Agent asks | HTTP requests | Events returned |
|---|---|---|
limit: 5 |
200 | 1000 |
limit: 100 |
10 | 1000 |
limit: 1000 |
1 | 1000 |
The parameter is inverted — a smaller limit means more work and the same full result. An agent that sets limit: 5 to keep its context small, which is exactly what the description tells it to expect, gets 1000 events and 200 round trips instead.
Splitting the two ideas fixes it cleanly: keep page size as an internal constant, and let limit cap the events actually returned. That also resolves the truncation-end point (guild_client.py:355) in the same edit.
Confirmed fixed
All six points from my last review, each verified by running the code:
extract_outputnoise — all three cases I reported now return'', and a genuinely-new answer type still surfaces.get_session_eventspagination — the answer is found across 4 / 250 / 1000-event transcripts, with all three stop conditions present (empty page,has_morefalse, hard cap).- Coverage — I ran the suite: 100 passed.
validateConfignow covers all five warning branches,beginGlobalcovers the env fallbacks and clamps, and all three tool functions are tested. Contract suite still 298 passed. - The module header,
config_int,_ENV_ATTR_MAP, the status-code constants, andexamples/README.md.
You also caught a wrong endpoint path in the module header ({owner}/{workspace} → {owner}~{workspace}) that I had missed. Good.
Non-blocking
guild_client.py:355— the cap keeps the oldest 1000 events, but the transcript is oldest-first so the answer is at the other end. A 1400-event transcript returns''. It fails empty rather than wrong, which is right, but silently.guild_client.py:358— the_has_moredocstring promises a fallback the code does not have when there is nopaginationblock.guild_client.py:143— the noise deny-list matches exact strings while its comment says the "runtime_*/llm_*families". Recording it as a decision, not asking for a change.nodes/test/framework/pipeline.py:36— the one change here that can reach other nodes, and nothing tests it. I checked the collision risk by hand and it is clear.
One thing that is not on you: CI does not run this test suite. There is no pytest job for nodes/ in ci.yml, pr-checks.yml, or _build.yaml, so those 100 tests only ran because I ran them locally. Flagging it so neither of us reads a green check as "the node tests passed".
The core is in good shape now. Fix the limit contract and its two docs, and I am happy.
Addresses the second-round review (asclearuc): the MAX_EVENT_LIMIT cap kept the oldest 1000 events, but the transcript is oldest-first so the answer is at the other end — a >1000-event transcript returned '' silently. - get_session_events now returns the MOST RECENT `limit` events (bounded by MAX_EVENT_LIMIT). When the API reports total_count > limit it seeks straight to the tail page; otherwise it pages forward, bounded, and trims to the tail. The answer (always last) is never dropped. Tests cover the seek path, the page-forward path, and the hard cap. - `limit` is now documented as "max events to return (most recent)", consistent across the tool schema and README, instead of an undocumented page size. - Fixed the `_has_more` docstring to match the code (no pagination block -> stop); split out `_total_count`. - Noise deny-list comment now says the types are matched exactly, not by family. - Added a test that the shared `_ENV_ATTR_MAP` change (KEY_ID/KEY_SECRET) resolves ROCKETRIDE_GUILD_* to the node's config fields. Tests: 102 unit + contract pass (verified under a forced ASCII locale). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks — the
On CI not running the node suite — you're right, and thanks for flagging it rather than letting the green check imply coverage. There is no pytest job for 102 unit + contract pass. |
|
Heads-up on the red checks at
This PR's own checks (Ruff, gitleaks) are green, and |
…diff) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
asclearuc
left a comment
asclearuc
left a comment
There was a problem hiding this comment.
Thanks @EdwardLien0426 — this is ready. You fixed the limit contract with a better design than the one I suggested, and the docs moved in the same commit.
Verdict: approved
Everything I raised across both rounds is fixed. I verified each one by running the code, not by reading the description.
limit now caps the result, and it costs less than before:
| Agent asks | Before | Now |
|---|---|---|
limit: 5 |
200 requests, 1000 events | 2 requests, 5 events |
limit: 100 |
10 requests, 1000 events | 2 requests, 100 events |
The tail is kept, not the head. A 5000-event transcript returns the last 100 and finds the answer. The 1400-event case that used to return '' now returns the answer in 2 requests. Seeking straight to the tail with total_count is better than the forward-paging fix I proposed — it makes the cost independent of transcript length.
Docs moved with the contract. Both the agent-facing description (IInstance.py:209) and README.md:85 now describe what the code does. The deny-list comment saying "matched exactly (not by prefix)" is the honest wording — thanks for picking the accurate option rather than widening the match.
Tests. I ran them, because CI has no pytest job for nodes/: 102 tool_guild + 314 contract = 416 passed. The new tests pin the tail-seek offset, the request-count bound, and the new _has_more signature. test_credential_env_vars_map_to_config_fields also closes my earlier point that the shared _ENV_ATTR_MAP change had nothing exercising it.
CI. Ruff, gitleaks, Shell API contract, and the macOS build pass. The Ubuntu and Windows builds were still running when I approved — they are the upstream onnxruntime pin and npm flake you already called out, not this diff. Please glance at them before merging.
Two notes, neither blocking
Both are inline. Neither is reachable against real Guild, so treat them as cleanup whenever you next touch this file — not as merge conditions.
I also resolved my own outdated review threads on this PR, so what stays open is only what still applies. I did not touch anyone else's threads.
Nice work on this one. The safety design — budget claimed before the billable POST, POSTs never retried, an empty answer preferred over a wrong one — is what makes the node trustworthy, and it held up under every case I threw at it.
|
The Ubuntu/Windows builds are green now — I merged On the two inline nits — agreed, and I'm deferring both rather than push new commits onto an approved, green PR:
Both are unreachable against real Guild today, so they don't change behaviour on any live path. |
|
Resolved the eight open review threads to clear the merge block — all six from rounds 1–2 were fixed in commits (you confirmed each on approval), and the two |
2 participants
Summary
tool_guild— a dual["data","tool"]node that runs Guild.ai agents from a RocketRide pipeline or an agent (mirrorstool_n8nfor the dual-node structure andtool_v0for IGlobal/test conventions and raise-not-return error handling).answers/text/documents/table.run_agent/get_session/get_session_eventslet an agent delegate to and follow up on Guild sessions.Why this fits the codebase
Guild.ai is a control plane for AI agents with no data-processing model of its own. This node is the complement to a RocketRide pipeline: RocketRide does the data work (ingest, parse, chunk, embed, retrieve) and hands a well-formed payload to a governed Guild agent to perform the action. The dual data+tool shape follows the
tool_n8nprecedent for delegating to a remote runtime.Type
feature (new pipeline/agent-tool node) + docs + tests
Testing
nodes/test/test_tool_guild.py), including regression fixtures captured from a live Guild transcriptruff check/formatclean;gitleaksclean;pytest nodes/test/test_tool_guild.py82 pass (full suite 380;test_contracts.py298 pass); verified end-to-end against a live Guild workspace (api_trigger session over HTTP Basic auth; session id / status / event response shapes confirmed and pinned as fixtures)ROCKETRIDE:GENERATED:PARAMSmarker block./builder nodes:test-contracts— not completed locally; the builder fails atserver:setup-pip(a build-env bootstrap step, unrelated to this node) before tests run. Ran the contract pytest directly instead (298 pass). CI runs the builder path in a clean environment../builder nodes:test/nodes:test-full— not run (same local build-env blocker)./builder nodes:docs-generate— not run (feature branch; an empty marker pair is expected until develop)Checklist
${ROCKETRIDE_GUILD_*}placeholders)nodes/test/+examples/)Linked Issue
Closes #1688
Summary by CodeRabbit