New Components - loopquest#21304
Open
TomPhillipsLabs wants to merge 10 commits into
Open
Conversation
dannyroosevelt
moved this to Ready for PR Review
in Component (Source and Action) Backlog
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Collaborator
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
Collaborator
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a new LoopQuest Pipedream component with an app definition, task body builder, two actions, a verdict webhook source, and package metadata. ChangesLoopQuest component
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
Author
Auth details for the
|
| Slug | Label | Type | Required | Secret | Default |
|---|---|---|---|---|---|
api_key |
API Key | string | yes | yes | — |
base_url |
Base URL | string | no | no | https://loopquest.tomphillips.uk |
Usage: sent on every request as Authorization: Bearer {{api_key}}.
Connection test / verification:
GET {{base_url}}/api/v1/me
Authorization: Bearer {{api_key}}
→ 200 { "org_id": "...", "workspace": "..." } (valid key)
→ 401 (invalid key)
Test key for validation (a dedicated, disposable review workspace — please feel free to use it to test all three components; I'll delete it after the review):
lq_H5_KpmUXndqXoM2hb1d1S8SLRrWfQDcV
Docs: https://loopquest.tomphillips.uk/docs · OpenAPI: https://loopquest.tomphillips.uk/openapi.json · Website: https://loopquest.tomphillips.uk
Happy to provide a logo asset in any size you need. Thanks!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@components/loopquest/actions/create-review-task/create-review-task.mjs`:
- Line 6: The action description text in create-review-task.mjs uses the wrong
documentation link label; update the description string in the createReviewTask
metadata so the markdown link text reads “[See the documentation]” instead of
“[See the docs]”, while keeping the same URL and surrounding sentence unchanged.
- Around line 3-45: The Create Review Task action is missing the required
annotations metadata on its exported object. Update the default export in
create-review-task.mjs to include an annotations object with readOnlyHint set to
false, destructiveHint set to false, and openWorldHint set to true, keeping the
rest of the action definition unchanged.
In `@components/loopquest/actions/get-task-status/get-task-status.mjs`:
- Line 6: The action description in get-task-status.mjs uses the wrong
documentation link phrasing. Update the description string in the LoopQuest task
status action to match the shared convention used by create-review-task.mjs,
replacing the end text with "[See the documentation]" while keeping the rest of
the description unchanged.
- Around line 3-18: The Get Task Status action is missing the required
annotations metadata. Update the action object in get-task-status.mjs to add an
annotations object on the exported definition, matching the pattern used by
create-review-task, and set readOnlyHint to true, destructiveHint to false, and
openWorldHint to true. Keep the change alongside the existing
loopquest-get-task-status action config so the hints are declared on the action
itself.
In `@components/loopquest/common/body.mjs`:
- Around line 13-18: In body.mjs, the manual truthiness checks around optional
request fields are unnecessary because axios drops undefined values
automatically. Update the body-building logic in the body assembly block to
assign p.source, p.externalId, p.callbackUrl, p.timeoutSeconds, p.onTimeout, and
p.reviewsRequired directly to body.source, body.external_id, body.callback_url,
body.timeout_seconds, body.on_timeout, and body.reviews_required without
wrapping them in if statements.
In `@components/loopquest/loopquest.app.mjs`:
- Line 9: Add missing description fields to the propDefinitions in
loopquest.app.mjs for title, reviewsRequired, and taskId. Update the
propDefinition entries near the existing labels so each includes a concise
description, and make taskId’s description explicitly tell users it is the id
returned by createTask and used by get-task-status. Use the existing
propDefinition objects and related symbols like createTask and get-task-status
to place the guidance correctly.
- Around line 73-87: The public API methods in loopquest.app.mjs use mixed
calling conventions, with createTask/getTask accepting destructured option
objects while subscribeVerdicts/unsubscribeVerdicts use positional arguments.
Update subscribeVerdicts and unsubscribeVerdicts to match the same destructured
signature style as the other methods, and adjust their internal references to
use the named fields consistently.
- Around line 57-87: The public methods createTask, getTask, subscribeVerdicts,
and unsubscribeVerdicts are duplicating raw axios request setup instead of using
the shared request helper. Refactor these methods in loopquest.app.mjs to
delegate to the existing private _makeRequest() (or add one if needed) so
headers, base URL, and axios options are centralized. Keep the method-specific
pieces only in the call site by passing the HTTP method, path, and payload
through the helper. Ensure the helper is the single place that constructs the
axios($, ...) call.
In `@components/loopquest/sources/new-verdict/new-verdict.mjs`:
- Around line 34-44: The event dedupe key in the new-verdict handler is too
coarse because `this.$emit` uses only `body.task_id`, which can collapse
distinct LoopQuest deliveries like escalation and final resolution into one.
Update the `new-verdict` flow so the `id` is unique per delivery by combining
`body.task_id` with the computed `verdict`/state, or by using a
delivery-specific payload field if available, while keeping it stable for the
same event.
- Around line 18-25: Direct `this.db.set`/`this.db.get` usage in `activate()`
and `deactivate()` violates the state access pattern for `$.service.db`. Add
private helper methods on the same service (for example `_setHookId()` and
`_getHookId()`) and move the `hookId` read/write logic there, then update
`activate()`/`deactivate()` to call those helpers instead of touching `db`
directly.
🪄 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
Run ID: 88a94710-cad3-40ff-bda8-e0ef7af4bf63
📒 Files selected for processing (6)
components/loopquest/actions/create-review-task/create-review-task.mjscomponents/loopquest/actions/get-task-status/get-task-status.mjscomponents/loopquest/common/body.mjscomponents/loopquest/loopquest.app.mjscomponents/loopquest/package.jsoncomponents/loopquest/sources/new-verdict/new-verdict.mjs
Comment on lines
+13
to
+18
| if (p.source) body.source = p.source; | ||
| if (p.externalId) body.external_id = p.externalId; | ||
| if (p.callbackUrl) body.callback_url = p.callbackUrl; | ||
| if (p.timeoutSeconds) body.timeout_seconds = p.timeoutSeconds; | ||
| if (p.onTimeout) body.on_timeout = p.onTimeout; | ||
| if (p.reviewsRequired) body.reviews_required = p.reviewsRequired; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Manual truthiness guards are unnecessary — rely on automatic undefined stripping.
Since these values are sent via @pipedream/platform's axios as a JSON data payload, undefined fields are dropped automatically during serialization. The if (p.x) body.x = ... guards can be replaced with direct unconditional assignment, per the guideline that optional props may be passed directly into request bodies without truthiness checks.
♻️ Proposed simplification
- if (p.source) body.source = p.source;
- if (p.externalId) body.external_id = p.externalId;
- if (p.callbackUrl) body.callback_url = p.callbackUrl;
- if (p.timeoutSeconds) body.timeout_seconds = p.timeoutSeconds;
- if (p.onTimeout) body.on_timeout = p.onTimeout;
- if (p.reviewsRequired) body.reviews_required = p.reviewsRequired;
+ body.source = p.source;
+ body.external_id = p.externalId;
+ body.callback_url = p.callbackUrl;
+ body.timeout_seconds = p.timeoutSeconds;
+ body.on_timeout = p.onTimeout;
+ body.reviews_required = p.reviewsRequired;📝 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.
Suggested change
| if (p.source) body.source = p.source; | |
| if (p.externalId) body.external_id = p.externalId; | |
| if (p.callbackUrl) body.callback_url = p.callbackUrl; | |
| if (p.timeoutSeconds) body.timeout_seconds = p.timeoutSeconds; | |
| if (p.onTimeout) body.on_timeout = p.onTimeout; | |
| if (p.reviewsRequired) body.reviews_required = p.reviewsRequired; | |
| body.source = p.source; | |
| body.external_id = p.externalId; | |
| body.callback_url = p.callbackUrl; | |
| body.timeout_seconds = p.timeoutSeconds; | |
| body.on_timeout = p.onTimeout; | |
| body.reviews_required = p.reviewsRequired; |
🤖 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 `@components/loopquest/common/body.mjs` around lines 13 - 18, In body.mjs, the
manual truthiness checks around optional request fields are unnecessary because
axios drops undefined values automatically. Update the body-building logic in
the body assembly block to assign p.source, p.externalId, p.callbackUrl,
p.timeoutSeconds, p.onTimeout, and p.reviewsRequired directly to body.source,
body.external_id, body.callback_url, body.timeout_seconds, body.on_timeout, and
body.reviews_required without wrapping them in if statements.
Source: Coding guidelines
Comment on lines
+18
to
+25
| async activate() { | ||
| const { id } = await this.loopquest.subscribeVerdicts(this, this.http.endpoint); | ||
| this.db.set("hookId", id); | ||
| }, | ||
| async deactivate() { | ||
| const hookId = this.db.get("hookId"); | ||
| if (hookId) await this.loopquest.unsubscribeVerdicts(this, hookId); | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Encapsulate db access in private helper methods.
this.db.set/this.db.get are called directly inside activate()/deactivate(). Per path instructions, state stored in $.service.db should go through private helper methods rather than being called directly from event handlers.
♻️ Proposed refactor
+ methods: {
+ _getHookId() {
+ return this.db.get("hookId");
+ },
+ _setHookId(id) {
+ this.db.set("hookId", id);
+ },
+ },
hooks: {
async activate() {
const { id } = await this.loopquest.subscribeVerdicts(this, this.http.endpoint);
- this.db.set("hookId", id);
+ this._setHookId(id);
},
async deactivate() {
- const hookId = this.db.get("hookId");
+ const hookId = this._getHookId();
if (hookId) await this.loopquest.unsubscribeVerdicts(this, hookId);
},
},As per path instructions, "State stored in $.service.db should be accessed through private methods (_getX(), _setX()), not called directly from run() or event handlers."
📝 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.
Suggested change
| async activate() { | |
| const { id } = await this.loopquest.subscribeVerdicts(this, this.http.endpoint); | |
| this.db.set("hookId", id); | |
| }, | |
| async deactivate() { | |
| const hookId = this.db.get("hookId"); | |
| if (hookId) await this.loopquest.unsubscribeVerdicts(this, hookId); | |
| }, | |
| methods: { | |
| _getHookId() { | |
| return this.db.get("hookId"); | |
| }, | |
| _setHookId(id) { | |
| this.db.set("hookId", id); | |
| }, | |
| }, | |
| async activate() { | |
| const { id } = await this.loopquest.subscribeVerdicts(this, this.http.endpoint); | |
| this._setHookId(id); | |
| }, | |
| async deactivate() { | |
| const hookId = this._getHookId(); | |
| if (hookId) await this.loopquest.unsubscribeVerdicts(this, hookId); | |
| }, |
🤖 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 `@components/loopquest/sources/new-verdict/new-verdict.mjs` around lines 18 -
25, Direct `this.db.set`/`this.db.get` usage in `activate()` and `deactivate()`
violates the state access pattern for `$.service.db`. Add private helper methods
on the same service (for example `_setHookId()` and `_getHookId()`) and move the
`hookId` read/write logic there, then update `activate()`/`deactivate()` to call
those helpers instead of touching `db` directly.
Source: Path instructions
Author
|
Thanks @coderabbitai — addressed 8 of the 10 in the latest commits:
Skipped 2, with reasoning:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
The 8 fixes all sound solid — the shared On the 2 you skipped, both justifications are reasonable and I won't push further:
Nice work getting this PR into shape. Let me know if you'd like another pass once the auth details are wired up on the platform side, or if there's anything else you'd like me to look at. 🐇✨ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LoopQuest
Adds components for LoopQuest — a human-in-the-loop review layer for AI/automation output. Send an item to a person to review, gate a downstream action until it's approved, or monitor quality in the background.
Components
POST /api/v1/tasks. Pick a review game (swiper, versus, sorter, detective, fixer, redact, grounding) and a mode (gateblocks until approval,monitorreviews out-of-band), with an optional timeout + on-timeout fallback.POST /api/v1/hooks) on deploy and unsubscribes on teardown; emits each resolved verdict deduped bytask_id.GET /api/v1/tasks/{id}.Auth
API key (
api_key), sent asAuthorization: Bearer. Connection test hitsGET /api/v1/me. Optionalbase_urlfor self-hosted deployments.Notes
loopquestapp will need registering on the platform (happy to provide auth config / a PD key as needed).0.0.1.Summary by CodeRabbit