Conversation
…in final blueprint
…ates' into GENIE-1171/STORY/Create-template-system
📝 WalkthroughWalkthroughAdds a new Agentic Templates feature: frontend page, components, hook, API clients and types; integrates validation into workspace; theme tokens and sidebar route; backend template service, Flask endpoints, and structured template errors and materialization flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as AgenticTemplates Page
participant Hook as useTemplates
participant API as Frontend API (/templates)
participant Server as Templates Service (Flask)
participant DB as Backend Storage
User->>UI: open templates catalog
UI->>Hook: fetchTemplates()
Hook->>API: GET /templates.list
API->>Server: GET /templates.list
Server->>DB: query templates
DB-->>Server: templates[]
Server-->>API: templates[]
API-->>Hook: templates[]
Hook-->>UI: render catalog
User->>UI: select template
UI->>Hook: fetchTemplateDetail(id)
Hook->>API: GET /template.get and /template.schema.get
API->>Server: GET template detail + schema
Server->>DB: retrieve template + schema
DB-->>Server: detail, schema
Server-->>API: detail, schema
API-->>Hook: detail, schema
Hook->>Hook: normalize schema -> normalizedFields
Hook-->>UI: show detail view
User->>UI: submit form to instantiate
UI->>Hook: validateTemplateInput(input)
Hook->>API: POST /template.input.validate
API->>Server: validate input
Server-->>API: validation result
alt valid
Hook->>API: POST /template.materialize
API->>Server: materialize request
Server->>DB: create blueprint/resources
DB-->>Server: blueprint_id, resources
Server-->>API: materialize response
API-->>Hook: success
Hook-->>UI: show InstantiationProgress (completed)
else invalid
API-->>Hook: validation errors
Hook-->>UI: show InstantiationProgress (failed)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
odaiodeh
requested review from
Lina-AbuYousef,
MayaCrmi,
sfiresht and
yhabushi79
This comment was marked as spam.
This comment was marked as spam.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@CodeRabbit review multi agent changes |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 2614 🏁 Script executed: Length of output: 351 🏁 Script executed: Length of output: 56 🏁 Script executed: Length of output: 29 🏁 Script executed: Length of output: 29 🏁 Script executed: Length of output: 105 🏁 Script executed: Length of output: 121 🏁 Script executed: Length of output: 121 🏁 Script executed: Length of output: 421 🏁 Script executed: Length of output: 280 🏁 Script executed: Length of output: 129 🏁 Script executed: Length of output: 210 🏁 Script executed: Length of output: 208 🏁 Script executed: Length of output: 236 🏁 Script executed: Length of output: 169 🏁 Script executed: Length of output: 19662 🏁 Script executed: Length of output: 17371 🏁 Script executed: Length of output: 3305 🏁 Script executed: Length of output: 3283 🏁 Script executed: Length of output: 5460 🏁 Script executed: Length of output: 10366 🏁 Script executed: Length of output: 5143 🏁 Script executed: Length of output: 7473 🏁 Script executed: Length of output: 3930 🏁 Script executed: Length of output: 11532
🎯 Architecture OverviewThe Templates system adds reusable blueprint templates with excellent separation of concerns: ✅ Strengths1. SOLID Principles Throughout
2. Clean Separation of Layers3. Rollback-on-Failure PatternThe saved_rids: List[str] = []
try:
saved_rids = self._save_resources(collected, user_id)
final_draft = self._build_final_draft(draft, id_mapping)
except Exception as e:
self._rollback(saved_rids) # ✅ Cleans up on failure
raise MaterializationError(...)4. Dynamic Pydantic Model CreationThe
|
…ates' into GENIE-1171/STORY/Create-template-system
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
- Code improvment | API call from external component instead of inline - Reset Logic | Change
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ates' into GENIE-1171/STORY/Create-template-system
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@multi-agent/api/flask/endpoints/templates.py`:
- Around line 28-58: The endpoint list_templates accepts a tags query param but
never uses it; update the call to svc.list_template_summaries in list_templates
to pass tags=tags (or transform tags as needed) and propagate that parameter
through the template_service method(s) (e.g.,
TemplateService.list_template_summaries and any downstream repository/query
functions) so tag filtering is actually applied; ensure signatures and callers
are updated to accept and use the tags argument.
- Around line 300-355: The endpoint materialize_template currently trusts the
body-supplied user_id; update it to derive and validate the caller from the
authentication context before calling
current_app.container.template_service.materialize: retrieve the authenticated
principal (e.g. from Flask's request context, session, or current_user), if an
authenticated id exists use that as the effective user_id, and if the request
body includes a user_id ensure it exactly matches the authenticated id
(otherwise return 401/403); then pass the validated/derived user_id into
svc.materialize and keep existing exception handling. Ensure the same validation
pattern is applied consistently to other handlers that accept a user_id
parameter.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@multi-agent/templates/errors.py`:
- Around line 88-102: The to_dict_list serialization in MaterializationError
currently calls e.model_dump() which can leave Enum fields as objects; update
MaterializationError.to_dict_list to call e.model_dump(mode="json") so Enum
values are serialized as JSON-safe strings, and make the identical change for
the MergeError/merge error serialization path (e.g., any MergeError.to_dict_list
or places calling model_dump on MergeFieldError/MergeError) to use
model_dump(mode="json") as well to ensure error_type Enums become strings in API
responses.
- Around line 120-144: Change the InstantiationError constructor signature to
use an explicit Optional[List[Any]] for the errors parameter (update the typing
import if necessary) and ensure _serialize calls model_dump(mode="json") when
serializing Pydantic models; keep the existing to_dict_list and _serialize flow
but replace model_dump() with model_dump(mode="json") and update the type hint
on __init__ (errors: Optional[List[Any]] = None).
- Around line 33-47: MergeError.to_dict_list currently calls e.model_dump()
without specifying JSON mode; update MergeError.to_dict_list and the analogous
MaterializationError.to_dict_list to call e.model_dump(mode="json") so
enum/complex types are serialized consistently for API responses. Locate the
methods named to_dict_list on the MergeError and MaterializationError classes
and change their model_dump() invocations to model_dump(mode="json") to match
the established serialization pattern.
In `@multi-agent/templates/service.py`:
- Around line 269-327: In materialize, ensure instantiation errors keep the
documented InstantiationError contract and that resource intent isn't silently
ignored: wrap the call to self._instantiator.instantiate(...) in a try/except
that catches MergeError and re-raises it as InstantiationError (preserving the
original exception as the __cause__), and before calling self._save_blueprint
check if save_resources is True and self._resources_service is None and raise a
RuntimeError (e.g. "ResourcesService not configured") instead of letting
_save_blueprint silently skip or proceed; references: materialize,
_instantiator.instantiate, MergeError, InstantiationError, _resources_service,
and _save_blueprint.
🧹 Nitpick comments (1)
multi-agent/templates/service.py (1)
212-237: Avoid catching broad exceptions during validation.
CatchingExceptioncan mask unexpected failures. Prefer handlingValidationErroronly and let other errors bubble.🔧 Proposed fix
+from pydantic import ValidationError @@ - try: - input_model(**user_input) - return InputValidationResult(is_valid=True, errors=[]) - except Exception as e: - return InputValidationResult(is_valid=False, errors=[str(e)]) + try: + input_model(**user_input) + return InputValidationResult(is_valid=True, errors=[]) + except ValidationError as e: + return InputValidationResult(is_valid=False, errors=[str(e)])
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@multi-agent/api/flask/endpoints/templates.py`:
- Around line 184-216: The delete_template endpoint on templates_bp currently
performs no authorization; before calling
current_app.container.template_service.exists or .delete_template you must
enforce access control: require an auth decorator or retrieve the authenticated
user (e.g., current_user or current_app.container.auth.get_current_user()) and
validate that the user is authorized to delete the template (owner,
has_role('admin'), or permitted by template_service.check_permission). If using
middleware, apply the auth decorator to templates_bp.route("/template.delete")
or, if enforcing inline, fetch the user and call a permission check on
template_service (e.g., template_service.user_can_delete(template_id, user_id))
and return 403 when unauthorized; only then call
svc.delete_template(template_id).
🧹 Nitpick comments (5)
multi-agent/templates/service.py (4)
101-102:datetime.utcnow()is deprecated.
datetime.utcnow()is deprecated as of Python 3.12 and returns a naive datetime. Consider usingdatetime.now(timezone.utc)for timezone-aware timestamps.♻️ Suggested fix
+from datetime import datetime, timezone -from datetime import datetime ... - created_at=datetime.utcnow(), - updated_at=datetime.utcnow(), + created_at=datetime.now(timezone.utc), + updated_at=datetime.now(timezone.utc),
113-129: Add exception chaining withfrom err.When re-raising exceptions, use
from errto preserve the original traceback for debugging. This is flagged by Ruff B904.♻️ Suggested fix
def get_template(self, template_id: str) -> Template: try: return self._repo.get(template_id) - except KeyError: - raise TemplateNotFoundError(template_id) + except KeyError as err: + raise TemplateNotFoundError(template_id) from err def update_template(self, template: Template) -> bool: try: - template.updated_at = datetime.utcnow() + template.updated_at = datetime.now(timezone.utc) return self._repo.update(template) - except KeyError: - raise TemplateNotFoundError(template.template_id) + except KeyError as err: + raise TemplateNotFoundError(template.template_id) from err
233-237: Consider catchingpydantic.ValidationErrorspecifically.Catching bare
Exceptionhere works but is overly broad. Sinceinput_modelis a Pydantic model, catchingpydantic.ValidationErrorwould be more precise and avoid masking unexpected errors.♻️ Suggested fix
+from pydantic import ValidationError ... try: input_model(**user_input) return InputValidationResult(is_valid=True, errors=[]) - except Exception as e: + except ValidationError as e: return InputValidationResult(is_valid=False, errors=[str(e)])
259-264: Add exception chaining withfrom err.Preserve the original exception context for better debugging. Also, use
{e!s}instead of{str(e)}for the conversion.♻️ Suggested fix
try: return self._instantiator.instantiate(template, user_input) except MergeError as e: - raise InstantiationError(str(e), errors=e.errors) + raise InstantiationError(str(e), errors=e.errors) from e except Exception as e: - raise InstantiationError(f"Instantiation failed: {str(e)}") + raise InstantiationError(f"Instantiation failed: {e!s}") from emulti-agent/api/flask/endpoints/templates.py (1)
250-250: Parameterinputshadows Python built-in.The parameter name
inputshadows Python's built-ininput()function. Consider renaming touser_inputorinput_datafor clarity. This pattern also appears ininstantiate_templateandmaterialize_template.♻️ Suggested fix for this endpoint
-def validate_template_input(template_id, input): +def validate_template_input(template_id, user_input): ... - result = svc.validate_input(template_id, input) + result = svc.validate_input(template_id, user_input)Note: The
@from_bodydecorator uses"input"as the key, which is fine for the JSON field name. Only the Python parameter needs renaming.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@multi-agent/templates/service.py`:
- Around line 131-137: delete_template currently calls
self._repo.delete(template_id) directly but does not catch the KeyError the
repository can raise; wrap the call in a try/except around
TemplateService.delete_template and catch KeyError from
TemplateRepository.delete, then raise TemplateNotFoundError(template_id) (same
pattern used in get_template/update_template) so not-found cases map to
TemplateNotFoundError instead of leaking a 500.
🧹 Nitpick comments (2)
multi-agent/templates/service.py (2)
259-264: Use exception chaining to preserve root cause when wrapping instantiation errors.Exception chaining with
raise ... from epreserves the original exception in the traceback for debugging without changing the public error contract (InstantiationError).♻️ Proposed fix
try: return self._instantiator.instantiate(template, user_input) except MergeError as e: - raise InstantiationError(str(e), errors=e.errors) + raise InstantiationError(str(e), errors=e.errors) from e except Exception as e: - raise InstantiationError(f"Instantiation failed: {str(e)}") + raise InstantiationError(f"Instantiation failed: {e}") from e
233-237: Narrow validation exceptions to PydanticValidationError.Catching
Exceptionmasks validation failures as user input errors when they should be distinguished from unexpected internal errors. Sinceinput_model(**user_input)only raisesValidationErroron invalid input, narrow the exception handler to catch only that. This allows other errors to bubble up for proper logging and debugging.♻️ Proposed fix
+from pydantic import ValidationError @@ try: input_model(**user_input) return InputValidationResult(is_valid=True, errors=[]) - except Exception as e: - return InputValidationResult(is_valid=False, errors=[str(e)]) + except ValidationError as e: + return InputValidationResult( + is_valid=False, + errors=[err.get("msg", str(err)) for err in e.errors()], + )
… blueprints, resources, sessions)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@multi-agent/api/flask/endpoints/resources.py`:
- Around line 161-206: The route handler validate_resources currently caps
max_workers with min(max_workers, 20) but doesn't prevent zero or negative
values; change the clamping so max_workers is at least 1 and at most 20 (e.g.
set max_workers = max(1, min(max_workers, 20)) or check if max_workers <= 0 then
assign 1) before calling svc.validate_resources to avoid breaking executor setup
downstream.
🧹 Nitpick comments (2)
multi-agent/api/flask/endpoints/sessions.py (1)
142-157: Acknowledged: Authorization check is missing for session deletion.The TODO correctly identifies a security gap. Currently, any caller with a valid
session_idcan delete sessions without ownership verification. This should be addressed before production deployment.To implement authorization, consider:
- Adding a
user_idparameter to the endpoint (similar to other session endpoints)- Verifying the session belongs to the requesting user before deletion
- Returning 403 Forbidden if the user doesn't own the session
Would you like me to generate a proposed implementation for the authorization check, or open a new issue to track this work?
multi-agent/templates/service.py (1)
216-242: Narrow validation exception handling to ValidationError.
Catching all exceptions hides internal failures (e.g., schema construction issues). Prefer catching Pydantic’s validation errors and let unexpected errors surface.🛠️ Suggested fix
+from pydantic import ValidationError @@ - try: - input_model(**user_input) - return InputValidationResult(is_valid=True, errors=[]) - except Exception as e: - return InputValidationResult(is_valid=False, errors=[str(e)]) + try: + input_model(**user_input) + return InputValidationResult(is_valid=True, errors=[]) + except ValidationError as e: + return InputValidationResult(is_valid=False, errors=[str(e)])
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…hat-community-ai-tools/UnifAI into GENIE-1167/EPIC/Agentic-AI-Templates
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@multi-agent/api/flask/endpoints/resources.py`:
- Around line 107-125: The delete_resource endpoint currently deletes by
resource_id without verifying ownership; update it to enforce that the
requesting user owns the resource by obtaining the current user's id from the
request context (e.g., g.current_user.id or your auth container) and either (A)
call resources_service.delete(resource_id, user_id) and implement the ownership
check inside ResourcesService.delete (compare fetched_resource.user_id to
user_id and raise a 403/AuthorizationError if mismatched), or (B) fetch the
resource via resources_service.get(resource_id) in the endpoint, verify
resource.user_id matches the current user id, and only then call
resources_service.delete(resource_id); ensure to return 403 on unauthorized
attempts and keep existing error handling for
ResourceInUseError/KeyError/Exception.
🧹 Nitpick comments (1)
multi-agent/api/flask/endpoints/resources.py (1)
19-29: Avoid returning raw exception messages in 500 responses.Returning
str(e)can leak internal details. Prefer logging server-side and returning a generic error; consider applying the same pattern across endpoints for consistency.Suggested pattern
- except Exception as e: - return jsonify({"error": str(e)}), 500 + except Exception: + current_app.logger.exception("save_resource failed") + return jsonify({"error": "Internal server error"}), 500
…ates' into GENIE-1171/STORY/Create-template-system
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@ui/client/src/components/agentic-ai/templates/FieldInputs.tsx`:
- Around line 81-93: The remove icon button inside the Badge is missing an
accessible name; update the button rendered next to {item} (the element with
onClick={() => handleRemove(index)}) to provide an accessible label by adding an
aria-label (e.g., aria-label={`Remove ${item}`}) or by including a
visually-hidden text node describing the action tied to the X icon component,
ensuring screen readers can identify the control.
- Around line 119-125: The button that toggles secret visibility (uses
showSecret, setShowSecret and renders Eye/EyeOff) is icon-only and lacks an
accessible name; update the button to provide a dynamic accessible label such as
aria-label={showSecret ? "Hide secret" : "Show secret"} (or aria-pressed and an
appropriate aria-label) so screen readers announce its purpose, and keep the
existing onClick behavior and visual icons (Eye/EyeOff) intact.
- Around line 184-194: The numeric case in FieldInputs.tsx currently converts
the input via Number(e.target.value) which can produce NaN for intermediate
inputs; update the onChange handler for the Input component (the case 'number'
branch) to parse the string safely: trim the input, attempt conversion, check
Number.isFinite or !Number.isNaN before calling onChange, and if the parse
yields NaN or the string is empty/invalid, call onChange(undefined) (or preserve
previous valid state if desired). Ensure you reference the Input component's
onChange and the surrounding field/value props so invalid numeric entries never
propagate NaN into payloads.
In `@ui/client/src/hooks/use-templates.ts`:
- Around line 338-353: The code treats any HTTP 200 response from
materializeTemplate as success without checking the MaterializeResponse.status
field; update the post-call logic in the function that calls materializeTemplate
to inspect result.status (from MaterializeResponse) and only mark success when
result.status === 'success'. On error (result.status === 'error')
setInstantiationStatus('failed'), setInstantiationResult(result) or null as
appropriate, show an error toast with result.message or a descriptive message,
and throw or return an error to halt further success handling (do not show the
current success toast). Ensure you update the branches around
setInstantiationStatus, setInstantiationResult, toast, and the returned value so
they depend on result.status.
🧹 Nitpick comments (1)
ui/client/src/hooks/use-templates.ts (1)
305-311: Clear prior validation state before starting a new materialization.Without resetting, stale errors can persist across retries.
🛠️ Suggested fix
try { setInstantiationStatus('validating'); setError(null); + setValidationErrors([]); + setElementValidationResults([]); + setInstantiationResult(null);
| <Badge | ||
| key={index} | ||
| variant="secondary" | ||
| className="bg-primary/20 text-primary border-primary/30 flex items-center gap-1" | ||
| > | ||
| {item} | ||
| <button | ||
| type="button" | ||
| onClick={() => handleRemove(index)} | ||
| className="ml-1 hover:text-red-400" | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label to the remove icon button.
The icon-only remove control has no accessible name, which makes it opaque to screen readers.
🛠️ Suggested fix
<button
type="button"
onClick={() => handleRemove(index)}
className="ml-1 hover:text-red-400"
+ aria-label={`Remove ${item}`}
>
- <X className="h-3 w-3" />
+ <X className="h-3 w-3" aria-hidden="true" />
</button>📝 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.
| <Badge | |
| key={index} | |
| variant="secondary" | |
| className="bg-primary/20 text-primary border-primary/30 flex items-center gap-1" | |
| > | |
| {item} | |
| <button | |
| type="button" | |
| onClick={() => handleRemove(index)} | |
| className="ml-1 hover:text-red-400" | |
| > | |
| <X className="h-3 w-3" /> | |
| </button> | |
| <Badge | |
| key={index} | |
| variant="secondary" | |
| className="bg-primary/20 text-primary border-primary/30 flex items-center gap-1" | |
| > | |
| {item} | |
| <button | |
| type="button" | |
| onClick={() => handleRemove(index)} | |
| className="ml-1 hover:text-red-400" | |
| aria-label={`Remove ${item}`} | |
| > | |
| <X className="h-3 w-3" aria-hidden="true" /> | |
| </button> |
🤖 Prompt for AI Agents
In `@ui/client/src/components/agentic-ai/templates/FieldInputs.tsx` around lines
81 - 93, The remove icon button inside the Badge is missing an accessible name;
update the button rendered next to {item} (the element with onClick={() =>
handleRemove(index)}) to provide an accessible label by adding an aria-label
(e.g., aria-label={`Remove ${item}`}) or by including a visually-hidden text
node describing the action tied to the X icon component, ensuring screen readers
can identify the control.
| <button | ||
| type="button" | ||
| onClick={() => setShowSecret(!showSecret)} | ||
| className="absolute right-3 top-1/2 transform -translate-y-1/2 text-gray-400 hover:text-white" | ||
| > | ||
| {showSecret ? <EyeOff className="h-4 w-4" /> : <Eye className="h-4 w-4" />} | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label to the show/hide secret toggle.
This is another icon-only control without a readable name for assistive tech.
🛠️ Suggested fix
<button
type="button"
onClick={() => setShowSecret(!showSecret)}
className="absolute right-3 top-1/2 transform -translate-y-1/2 text-gray-400 hover:text-white"
+ aria-label={showSecret ? 'Hide secret' : 'Show secret'}
+ aria-pressed={showSecret}
>
- {showSecret ? <EyeOff className="h-4 w-4" /> : <Eye className="h-4 w-4" />}
+ {showSecret ? <EyeOff className="h-4 w-4" aria-hidden="true" /> : <Eye className="h-4 w-4" aria-hidden="true" />}
</button>📝 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.
| <button | |
| type="button" | |
| onClick={() => setShowSecret(!showSecret)} | |
| className="absolute right-3 top-1/2 transform -translate-y-1/2 text-gray-400 hover:text-white" | |
| > | |
| {showSecret ? <EyeOff className="h-4 w-4" /> : <Eye className="h-4 w-4" />} | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => setShowSecret(!showSecret)} | |
| className="absolute right-3 top-1/2 transform -translate-y-1/2 text-gray-400 hover:text-white" | |
| aria-label={showSecret ? 'Hide secret' : 'Show secret'} | |
| aria-pressed={showSecret} | |
| > | |
| {showSecret ? <EyeOff className="h-4 w-4" aria-hidden="true" /> : <Eye className="h-4 w-4" aria-hidden="true" />} | |
| </button> |
🤖 Prompt for AI Agents
In `@ui/client/src/components/agentic-ai/templates/FieldInputs.tsx` around lines
119 - 125, The button that toggles secret visibility (uses showSecret,
setShowSecret and renders Eye/EyeOff) is icon-only and lacks an accessible name;
update the button to provide a dynamic accessible label such as
aria-label={showSecret ? "Hide secret" : "Show secret"} (or aria-pressed and an
appropriate aria-label) so screen readers announce its purpose, and keep the
existing onClick behavior and visual icons (Eye/EyeOff) intact.
| case 'number': | ||
| return ( | ||
| <Input | ||
| type="number" | ||
| value={value ?? ''} | ||
| onChange={(e) => onChange(e.target.value ? Number(e.target.value) : undefined)} | ||
| placeholder={`Enter ${field.label.toLowerCase()}`} | ||
| className="bg-background-dark border-gray-700" | ||
| min={field.minimum} | ||
| max={field.maximum} | ||
| /> |
There was a problem hiding this comment.
Guard against NaN when parsing numeric input.
Number(e.target.value) can produce NaN (e.g., user types - or e), which then propagates into payloads.
🛠️ Suggested fix
<Input
type="number"
value={value ?? ''}
- onChange={(e) => onChange(e.target.value ? Number(e.target.value) : undefined)}
+ onChange={(e) => {
+ if (e.target.value === '') return onChange(undefined);
+ const parsed = Number(e.target.value);
+ onChange(Number.isFinite(parsed) ? parsed : undefined);
+ }}
placeholder={`Enter ${field.label.toLowerCase()}`}
className="bg-background-dark border-gray-700"
min={field.minimum}
max={field.maximum}
/>📝 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.
| case 'number': | |
| return ( | |
| <Input | |
| type="number" | |
| value={value ?? ''} | |
| onChange={(e) => onChange(e.target.value ? Number(e.target.value) : undefined)} | |
| placeholder={`Enter ${field.label.toLowerCase()}`} | |
| className="bg-background-dark border-gray-700" | |
| min={field.minimum} | |
| max={field.maximum} | |
| /> | |
| case 'number': | |
| return ( | |
| <Input | |
| type="number" | |
| value={value ?? ''} | |
| onChange={(e) => { | |
| if (e.target.value === '') return onChange(undefined); | |
| const parsed = Number(e.target.value); | |
| onChange(Number.isFinite(parsed) ? parsed : undefined); | |
| }} | |
| placeholder={`Enter ${field.label.toLowerCase()}`} | |
| className="bg-background-dark border-gray-700" | |
| min={field.minimum} | |
| max={field.maximum} | |
| /> |
🤖 Prompt for AI Agents
In `@ui/client/src/components/agentic-ai/templates/FieldInputs.tsx` around lines
184 - 194, The numeric case in FieldInputs.tsx currently converts the input via
Number(e.target.value) which can produce NaN for intermediate inputs; update the
onChange handler for the Input component (the case 'number' branch) to parse the
string safely: trim the input, attempt conversion, check Number.isFinite or
!Number.isNaN before calling onChange, and if the parse yields NaN or the string
is empty/invalid, call onChange(undefined) (or preserve previous valid state if
desired). Ensure you reference the Input component's onChange and the
surrounding field/value props so invalid numeric entries never propagate NaN
into payloads.
| const result = await materializeTemplate({ | ||
| templateId, | ||
| userId, | ||
| input, | ||
| blueprintName | ||
| }); | ||
|
|
||
| setInstantiationStatus('completed'); | ||
| setInstantiationResult(result); | ||
|
|
||
| toast({ | ||
| title: 'Success', | ||
| description: `Blueprint "${result.name}" created successfully!` | ||
| }); | ||
|
|
||
| return result; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and examine the file
find . -name "use-templates.ts" -type fRepository: redhat-community-ai-tools/UnifAI
Length of output: 114
🏁 Script executed:
# Let's also look at the general structure of the src directory
ls -la ui/client/src/Repository: redhat-community-ai-tools/UnifAI
Length of output: 1166
🏁 Script executed:
# Now let's read the use-templates.ts file to see the code at lines 338-353
wc -l ui/client/src/hooks/use-templates.tsRepository: redhat-community-ai-tools/UnifAI
Length of output: 116
🏁 Script executed:
# Read the file around lines 338-353
sed -n '330,360p' ui/client/src/hooks/use-templates.tsRepository: redhat-community-ai-tools/UnifAI
Length of output: 806
🏁 Script executed:
# Search for materializeTemplate function definition
rg -n "materializeTemplate" ui/client/src/ --type typescript --type tsx -A 3Repository: redhat-community-ai-tools/UnifAI
Length of output: 110
🏁 Script executed:
# Search for materializeTemplate function definition without specifying file type
rg -n "materializeTemplate" ui/client/src/ -A 3Repository: redhat-community-ai-tools/UnifAI
Length of output: 1040
🏁 Script executed:
# Now let's find the MaterializeResponse type definition
rg -n "MaterializeResponse\|MaterializeParams" ui/client/src/api/templates.ts -B 2 -A 10Repository: redhat-community-ai-tools/UnifAI
Length of output: 58
🏁 Script executed:
# Let's read the api/templates.ts file to see the type definitions
cat -n ui/client/src/api/templates.tsRepository: redhat-community-ai-tools/UnifAI
Length of output: 3421
🏁 Script executed:
# Now let's find the MaterializeResponse type definition
cat -n ui/client/src/types/templates.tsRepository: redhat-community-ai-tools/UnifAI
Length of output: 6920
Check the status field in the API response—the backend can return status: 'error' with HTTP 200.
The MaterializeResponse interface defines status: 'success' | 'error'. The current code treats any HTTP 200 response as success without checking this field, so users will see a success toast even if the operation failed.
🛠️ Suggested fix
const result = await materializeTemplate({
templateId,
userId,
input,
blueprintName
});
+ if (result.status === 'error') {
+ setError('Failed to materialize template');
+ setInstantiationStatus('failed');
+ toast({
+ title: 'Error',
+ description: 'Failed to materialize template',
+ variant: 'destructive'
+ });
+ return null;
+ }
+
setInstantiationStatus('completed');
setInstantiationResult(result);📝 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.
| const result = await materializeTemplate({ | |
| templateId, | |
| userId, | |
| input, | |
| blueprintName | |
| }); | |
| setInstantiationStatus('completed'); | |
| setInstantiationResult(result); | |
| toast({ | |
| title: 'Success', | |
| description: `Blueprint "${result.name}" created successfully!` | |
| }); | |
| return result; | |
| const result = await materializeTemplate({ | |
| templateId, | |
| userId, | |
| input, | |
| blueprintName | |
| }); | |
| if (result.status === 'error') { | |
| setError('Failed to materialize template'); | |
| setInstantiationStatus('failed'); | |
| toast({ | |
| title: 'Error', | |
| description: 'Failed to materialize template', | |
| variant: 'destructive' | |
| }); | |
| return null; | |
| } | |
| setInstantiationStatus('completed'); | |
| setInstantiationResult(result); | |
| toast({ | |
| title: 'Success', | |
| description: `Blueprint "${result.name}" created successfully!` | |
| }); | |
| return result; |
🤖 Prompt for AI Agents
In `@ui/client/src/hooks/use-templates.ts` around lines 338 - 353, The code treats
any HTTP 200 response from materializeTemplate as success without checking the
MaterializeResponse.status field; update the post-call logic in the function
that calls materializeTemplate to inspect result.status (from
MaterializeResponse) and only mark success when result.status === 'success'. On
error (result.status === 'error') setInstantiationStatus('failed'),
setInstantiationResult(result) or null as appropriate, show an error toast with
result.message or a descriptive message, and throw or return an error to halt
further success handling (do not show the current success toast). Ensure you
update the branches around setInstantiationStatus, setInstantiationResult,
toast, and the returned value so they depend on result.status.
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)
multi-agent/actions/providers/mcp/get_tools_names/get_tools_names.py (1)
2-18:⚠️ Potential issue | 🟠 MajorMask bearer_token in the action schema (SecretHint).
This is sensitive input; add SecretHint so UI masks it and avoids accidental exposure.
🔒 Proposed fix
-from pydantic import BaseModel, HttpUrl, Field +from pydantic import BaseModel, HttpUrl, Field +from core.field_hints import SecretHint @@ class GetToolsNamesInput(BaseActionInput): """Input for MCP tools discovery""" sse_endpoint: HttpUrl bearer_token: Optional[str] = Field( default=None, - description="Bearer token for MCP server authentication" + description="Bearer token for MCP server authentication", + json_schema_extra=SecretHint(reason="API credentials should be masked").to_hints() )
🤖 Fix all issues with AI agents
In
`@multi-agent/actions/providers/mcp/validate_connection/validate_connection.py`:
- Around line 7-29: The ValidateConnectionInput model exposes bearer_token
without SecretHint; update the pydantic field for
ValidateConnectionInput.bearer_token to include hint=SecretHint (matching
McpProviderConfig) so the UI treats it as sensitive—locate the
ValidateConnectionInput class and modify the bearer_token Field to pass the
SecretHint hint from core.field_hints.
| import anyio | ||
| import time | ||
| from typing import Optional, Dict, Any | ||
|
|
||
| from pydantic import HttpUrl | ||
| from pydantic import HttpUrl, Field | ||
|
|
||
| from actions.common.base_action import BaseAction | ||
| from actions.common.action_models import BaseActionInput, BaseActionOutput, ActionType | ||
| from elements.providers.mcp_server_client.mcp_server_client import McpServerClient | ||
| from elements.providers.mcp_server_client.mcp_provider_factory import McpProviderFactory | ||
| from elements.providers.mcp_server_client.config import McpProviderConfig | ||
| from elements.providers.mcp_server_client.identifiers import Identifier | ||
| from core.enums import ResourceCategory | ||
| from core.field_hints import SecretHint | ||
|
|
||
|
|
||
| # Input/Output models for this action | ||
| class ValidateConnectionInput(BaseActionInput): | ||
| """Input for MCP connection validation""" | ||
| sse_endpoint: HttpUrl | ||
| bearer_token: Optional[str] = Field( | ||
| default=None, | ||
| description="Bearer token for MCP server authentication" | ||
| ) |
There was a problem hiding this comment.
Mask bearer_token in the action schema (SecretHint).
The token is sensitive; without SecretHint the UI may render plaintext and risk exposure. Align this with McpProviderConfig by adding SecretHint hints.
🔒 Proposed fix
class ValidateConnectionInput(BaseActionInput):
"""Input for MCP connection validation"""
sse_endpoint: HttpUrl
bearer_token: Optional[str] = Field(
default=None,
- description="Bearer token for MCP server authentication"
+ description="Bearer token for MCP server authentication",
+ json_schema_extra=SecretHint(reason="API credentials should be masked").to_hints()
)🤖 Prompt for AI Agents
In `@multi-agent/actions/providers/mcp/validate_connection/validate_connection.py`
around lines 7 - 29, The ValidateConnectionInput model exposes bearer_token
without SecretHint; update the pydantic field for
ValidateConnectionInput.bearer_token to include hint=SecretHint (matching
McpProviderConfig) so the UI treats it as sensitive—locate the
ValidateConnectionInput class and modify the bearer_token Field to pass the
SecretHint hint from core.field_hints.
…ates' into GENIE-1171/STORY/Create-template-system
…ates' into GENIE-1171/STORY/Create-template-system
2 participants
## 📋 Templates Feature Reusable blueprint templates with placeholder support. Users browse templates, fill required fields, and get a fully configured blueprint with resources saved to their account. --- ## 🎯 What is a Template? ``` ╔═══════════════════════════════════════════════════════════════════════════════╗ ║ T E M P L A T E ║ ╠═══════════════════════════════════════════════════════════════════════════════╣ ║ ║ ║ ┌─────────────────────────────────────────────────────────────────────┐ ║ ║ │ 📄 BlueprintDraft │ ║ ║ │ │ ║ ║ │ A complete, valid blueprint with placeholder default values │ ║ ║ │ │ ║ ║ │ nodes: │ ║ ║ │ - orchestrator_node: │ ║ ║ │ llm: "google_gemini" │ ║ ║ │ system_prompt: "PLACEHOLDER_PROMPT" ◄── placeholder │ ║ ║ │ │ ║ ║ │ llms: │ ║ ║ │ - google_gemini: │ ║ ║ │ api_key: "PLACEHOLDER_KEY" ◄── placeholder │ ║ ║ │ model: "gemini-1.5-pro" │ ║ ║ └─────────────────────────────────────────────────────────────────────┘ ║ ║ + ║ ║ ┌─────────────────────────────────────────────────────────────────────┐ ║ ║ │ 🏷️ PlaceholderMeta │ ║ ║ │ │ ║ ║ │ Points to fields that need user input: │ ║ ║ │ │ ║ ║ │ • llms.google_gemini.api_key (required) │ ║ ║ │ • nodes.orchestrator.system_prompt (optional) │ ║ ║ └─────────────────────────────────────────────────────────────────────┘ ║ ║ + ║ ║ ┌─────────────────────────────────────────────────────────────────────┐ ║ ║ │ 📋 Metadata │ ║ ║ │ │ ║ ║ │ author, tags, version, category, is_public │ ║ ║ └─────────────────────────────────────────────────────────────────────┘ ║ ║ ║ ╚═══════════════════════════════════════════════════════════════════════════════╝ ``` --- ## 🔄 User Journey ``` ┌─────────┐ ┌─────────┐ ┌─────────┐ ┌─────────┐ │ BROWSE │ │ VIEW │ │ FILL │ │ CREATE │ │Templates│ ───► │ Schema │ ───► │ Inputs │ ───► │Blueprint│ └─────────┘ └─────────┘ └─────────┘ └─────────┘ │ │ │ │ │ │ │ │ ▼ ▼ ▼ ▼ ╭─────────╮ ╭─────────╮ ╭─────────╮ ╭─────────╮ │ list_ │ │ get_ │ │validate_│ │material-│ │templates│ │ input_ │ │ input() │ │ ize() │ │ () │ │schema() │ │ │ │ │ ╰─────────╯ ╰─────────╯ ╰─────────╯ ╰─────────╯ │ ▼ ┌────────────────┐ │ ✅ Blueprint │ │ ✅ Resources │ │ Saved! │ └────────────────┘ ``` --- ## 🔧 Materialization Pipeline ``` ┌──────────────────┐ │ USER INPUT │ │ {api_key: ...} │ └────────┬─────────┘ │ ▼ ╔════════════════════════════════════════════════════════════════════════════════╗ ║ STEP 1: INSTANTIATE ║ ╠════════════════════════════════════════════════════════════════════════════════╣ ║ ║ ║ Template Draft User Input Merged Draft ║ ║ ┌──────────────┐ + ┌──────────┐ = ┌──────────────┐ ║ ║ │ api_key: │ │ api_key: │ │ api_key: │ ║ ║ │ "PLACEHOLDER"│ │ "sk-..." │ │ "sk-..." │ ║ ║ └──────────────┘ └──────────┘ └──────────────┘ ║ ║ ║ ║ TemplateInstantiator.instantiate() ║ ╚════════════════════════════════════════════════════════════════════════════════╝ │ ▼ ╔════════════════════════════════════════════════════════════════════════════════╗ ║ STEP 2: VALIDATE ║ ╠════════════════════════════════════════════════════════════════════════════════╣ ║ ║ ║ BlueprintService.validate_draft() ║ ║ ✓ All configs pass Pydantic schemas ║ ║ ║ ╚════════════════════════════════════════════════════════════════════════════════╝ │ ▼ ╔════════════════════════════════════════════════════════════════════════════════╗ ║ STEP 3: MATERIALIZE ║ ╠════════════════════════════════════════════════════════════════════════════════╣ ║ ║ ║ ResourceMaterializer.materialize() ║ ║ ║ ║ ┌────────────────────────────────────────────────────────────────────────┐ ║ ║ │ │ ║ ║ │ 3a. COLLECT 3b. REMAP 3c. SAVE 3d. BUILD │ ║ ║ │ ┌─────────┐ ┌─────────┐ ┌─────────┐ ┌─────────┐ │ ║ ║ │ │ Find │ ──► │ Update │ ──► │ Save to │ ──► │ Create │ │ ║ ║ │ │ inline │ │ Ref IDs │ │ DB │ │ $ref │ │ ║ ║ │ │resources│ │ │ │ │ │ entries │ │ ║ ║ │ └─────────┘ └─────────┘ └─────────┘ └─────────┘ │ ║ ║ │ │ ║ ║ │ If any step fails ──► ROLLBACK (delete saved resources) │ ║ ║ │ │ ║ ║ └────────────────────────────────────────────────────────────────────────┘ ║ ╚════════════════════════════════════════════════════════════════════════════════╝ │ ▼ ╔════════════════════════════════════════════════════════════════════════════════╗ ║ STEP 4: SAVE BLUEPRINT ║ ╠════════════════════════════════════════════════════════════════════════════════╣ ║ ║ ║ BlueprintService.save_draft() ║ ║ → Blueprint with $ref entries saved to user's account ║ ║ ║ ╚════════════════════════════════════════════════════════════════════════════════╝ │ ▼ ┌───────────────────────┐ │ 📦 RESULT │ │ │ │ blueprint_id: "bp-1" │ │ resource_ids: [...] │ │ fields_filled: 3 │ └───────────────────────┘ ``` --- ## 🔀 Before → After Transformation ``` B E F O R E A F T E R (Template with inline configs) (Materialized with $refs) ┌────────────────────────────────────────┐ ┌────────────────────────────────────────┐ │ BlueprintDraft │ │ BlueprintDraft │ │ │ │ │ │ nodes: │ │ nodes: │ │ ┌──────────────────────────────────┐ │ │ ┌──────────────────────────────────┐ │ │ │ rid: "orchestrator" │ │ │ │ rid: "$ref:abc123" ◄── saved │ │ │ │ type: "orchestrator_node" │ │ │ └──────────────────────────────────┘ │ │ │ config: │ │ │ ┌──────────────────────────────────┐ │ │ │ llm: LLMRef("google_gemini") │ │ ─► │ │ rid: "user_question" │ │ │ │ system_prompt: "You are..." │ │ │ │ (system node - kept inline) │ │ │ └──────────────────────────────────┘ │ │ └──────────────────────────────────┘ │ │ ┌──────────────────────────────────┐ │ │ │ │ │ rid: "user_question" │ │ │ llms: [] ◄── empty (in node config) │ │ │ (system node) │ │ │ │ │ └──────────────────────────────────┘ │ │ plan: │ │ │ │ node: "abc123" ◄── updated │ │ llms: │ │ │ │ ┌──────────────────────────────────┐ │ └────────────────────────────────────────┘ │ │ rid: "google_gemini" │ │ │ │ config: │ │ │ │ api_key: "sk-..." │ │ DATABASE │ │ model: "gemini-1.5-pro" │ │ ┌────────────────────────────────────────┐ │ └──────────────────────────────────┘ │ │ │ │ │ │ Resource: abc123 │ │ plan: │ │ type: orchestrator_node │ │ node: "orchestrator" │ │ cfg: {llm: "$ref:def456", ...} │ │ │ │ │ └────────────────────────────────────────┘ │ Resource: def456 │ │ type: google_genai │ │ cfg: {api_key: "sk-...", ...} │ │ │ └────────────────────────────────────────┘ ``` --- ## 🏗️ Architecture ``` templates/ │ ├── service.py ◄── Public API (TemplateService) │ • create, get, list, search │ • get_input_schema, validate_input │ • instantiate, materialize │ ├── models/ │ └── template.py ◄── Data Models │ • Template, PlaceholderMeta │ • TemplateSummary, ResultModels │ ├── repository/ │ ├── repository.py ◄── Abstract interface │ └── mongo_repository.py ◄── MongoDB implementation │ ├── schema/ │ └── analyzer.py ◄── PlaceholderAnalyzer │ • Creates Pydantic input models │ • Generates JSON Schema for UI │ └── instantiation/ ├── models.py ◄── CollectedResource, Results ├── instantiator.py ◄── TemplateInstantiator │ • Merges user input into draft └── materializer.py ◄── ResourceMaterializer • Saves resources to DB • Creates $ref entries ``` --- ## ✨ Key Design Decisions | Decision | Why | |----------|-----| | 📄 **Draft is always valid** | Placeholder fields have defaults that pass Pydantic validation | | 🏷️ **Minimal PlaceholderMeta** | Just pointers; type info from ElementRegistry at runtime | | 🔄 **RefRemapper utility** | Clean, reusable Ref object remapping in `core/ref/` | | ↩️ **Rollback on failure** | If save fails, all saved resources are deleted | | 🎯 **Single rollback point** | One try-except in `materialize()` handles all failures | | 📦 **Separated models** | Data models in `models.py`, logic in class files | ```Summary by CodeRabbit
New Features
API
Hooks & Validation
Providers
Types & Styling