Conversation
WalkthroughThis update introduces several key enhancements and additions to the draive framework. A comprehensive new guide, "BasicStageUsage.md," documents the concept, usage, and advanced patterns for Stages within draive. The version number in ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
KaQuMiQ
force-pushed
the
feature/resource_uri
branch
2 times, most recently
from
eecb394 to
431ac39
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🧹 Nitpick comments (4)
guides/BasicStageUsage.md (1)
49-51: Minor grammar improvement“LMMOutput will be also used…” → “LMMOutput will also be used…”.
-LMMOutput will be also used as the result of this stage. +LMMOutput will also be used as the result of this stage.Purely stylistic – feel free to ignore if you prefer the current phrasing.
🧰 Tools
🪛 LanguageTool
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...(ALSO_PLACEMENT)
src/draive/parameters/validation.py (1)
870-878: Double-conversion ofint→boolcan be simplifiedMinor:
validated = bool(value != 0)already returns abool; the subsequentreturn validatedis enough – no need to create an extra variable.No functional impact, just readability.
src/draive/resources/template.py (2)
126-137: Discarding repeated query parameters may be surprising.
parse_qs()returns a list of values per key, but the implementation keeps only the first entry.
If a URI legitimately contains repeated keys (?tag=python&tag=ai) information is silently lost.Either:
- params[key] = values[0] + params[key] = values if len(values) > 1 else values[0]or expose the full list to the caller. At minimum, document this behaviour in the doc-string.
269-293: Public API change lacks deprecation shim.Changing the decorator from
uri=touri_template=is a breaking change for every consumer.
Consider keeping a deprecated alias for one release cycle:def resource[**Args]( *, - uri_template: str, + uri_template: str | None = None, + # deprecated — remove in vX.Y + uri: str | None = None, ... ): - def wrap( + if uri_template is None: + if uri is None: + raise TypeError("Either 'uri_template' or deprecated 'uri' must be supplied") + warnings.warn("'uri' is deprecated, use 'uri_template' instead", DeprecationWarning, stacklevel=2) + uri_template = uri + + def wrap(This avoids hard breaks and would have prevented the current test failures.
🛑 Comments failed to post (7)
src/draive/mcp/server.py (2)
184-190:
⚠️ Potential issue
AnyUrl()rejects URI templates containing{}
pydantic.AnyUrlperforms strict parsing and will raiseValidationErrorfor strings like/images/{id}or scheme-less paths. Converting the template toAnyUrltherefore explodes at startup.-uri=AnyUrl(resource.declaration.uri_template), +uri=resource.declaration.uri_template, # keep as plain strIf you need URI validation, use a relaxed type such as
pydantic.AnyHttpUrlafter expanding the template, or add a custom validator.Fail-fast exceptions here will prevent the server from starting.
📝 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.uri=resource.declaration.uri_template, # keep as plain str mimeType=resource.declaration.mime_type, name=resource.declaration.name, description=resource.declaration.description, ) ) available_resources[resource.declaration.uri_template] = resource🤖 Prompt for AI Agents
In src/draive/mcp/server.py around lines 184 to 190, the use of pydantic.AnyUrl to validate URI templates causes validation errors because AnyUrl rejects strings with curly braces like /images/{id}. To fix this, replace AnyUrl with a more relaxed type such as pydantic.AnyHttpUrl or a custom validator that allows URI templates, ensuring validation occurs after template expansion. This prevents startup failures due to strict URI parsing.
170-179: 🛠️ Refactor suggestion
⚠️ Potential issue
available_resourceswill never match real URIs for templated resourcesBy using the literal
resource.declaration.uri_template(e.g./images/{id}) as the dictionary key you cannot subsequently resolve/images/42inread_resource, because the lookup is still a directdict.get(uri_string).
There is a TODO a few lines below acknowledging this, but with the present change any templated resource with arguments becomes unreachable and will raise “Resource is not defined”.-available_resources[resource.declaration.uri_template] = resource +# key by the *compiled* template matcher or at least the raw object so we can +# evaluate `.matches(uri)` later. +available_resources[resource] = resource # or use resource.declaration.patternRecommendation:
- Keep a separate
templateslist/dict keyed by theResourceTemplateitself.- In
read_resource, iterate through templates and calltemplate.matches(uri)/resolve_from_uri.Without this fix, any client that relies on RFC-6570 expansion will fail.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/draive/mcp/server.py around lines 170 to 179, the current code uses the raw URI template string as a key in available_resources, which prevents matching actual URIs like /images/42. To fix this, maintain a separate collection (list or dict) of resource templates keyed by the ResourceTemplate object itself. Then, update read_resource to iterate over these templates, using their matches(uri) or resolve_from_uri methods to find and resolve the correct resource for a given URI. This ensures templated resources can be correctly matched and accessed.src/draive/parameters/validation.py (2)
726-744: 🛠️ Refactor suggestion
⚠️ Potential issue
_prepare_validator_of_floatsilently treatsTrue/Falseas numbersBecause
boolis a subclass ofint, thecase int():branch acceptsTrue/Falseand converts them to1.0/0.0.
This is almost never desirable for a float parameter.- match value: - case float(): + match value: + case bool(): # explicitly disallow bool + pass # fall-through to error + case float():Apply the same guard in the non-verifier branch.
📝 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.match value: case bool(): # explicitly disallow bool pass # fall-through to error case float(): verifier(value) return value case int(): validated = float(value) verifier(validated) return validated case str(): try: validated = float(value) except (ValueError, TypeError): pass # invalid else: verifier(validated)🤖 Prompt for AI Agents
In src/draive/parameters/validation.py around lines 726 to 744, the code treats boolean values as integers because bool is a subclass of int, causing True/False to be converted to 1.0/0.0, which is undesirable for float parameters. To fix this, add an explicit check to exclude bool values before processing the int case and similarly apply this guard in the branch where the verifier is not called, ensuring booleans are not silently accepted as floats.
795-822: 🛠️ Refactor suggestion
⚠️ Potential issue
_prepare_validator_of_intacceptsbool, which is usually unintended
case int():will match booleans (Trueis1,Falseis0).
If callers specifyintthey rarely expect boolean literals to be valid.- case int(): + case bool(): + pass # treat bools as invalid ints + case int():Repeat the guard in the branch without verifier.
📝 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 bool(): pass # treat bools as invalid ints case int(): verifier(value) return value case float(): validated = int(value) if value == validated: verifier(validated) return int(validated) # else invalid case str(): try: validated = int(value) except (ValueError, TypeError): pass # invalid else: verifier(validated) return validated case _: pass # invalid raise TypeError(f"'{value}' is not matching expected type of 'int'")🤖 Prompt for AI Agents
In src/draive/parameters/validation.py between lines 795 and 822, the case handling int() currently accepts boolean values because bool is a subclass of int. To fix this, add an explicit guard to exclude bool values in the int case branch before calling verifier. This means checking if the value is an int but not a bool, and only then proceed with verification and returning the value. Apply the same guard in the branch without verifier to ensure booleans are not accepted as valid ints.src/draive/resources/template.py (3)
96-99:
⚠️ Potential issue
resolve()still returns the raw template – implement real expansion.
ResourceTemplate.resolve()currently returnsself.declaration.uri_templatewith a TODO.
Down-stream callers will receive a URI containing placeholders (e.g.…/{user_id}), which breaks clients that expect concrete links.Either:
- Delegate to
urllib.parse.urlunparse+ the existing_extract_*helpers, or- Call the (new) inverse of
_extract_uri_parameters, filling inargs/kwargs.Failing to do so hides the bug until runtime and defeats the point of strong typing.
🤖 Prompt for AI Agents
In src/draive/resources/template.py around lines 96 to 99, the resolve() method currently returns the raw URI template with placeholders instead of a fully expanded URI. To fix this, implement real URI expansion by either using urllib.parse.urlunparse combined with the existing _extract_* helper methods to build the full URI, or create a method that inversely applies _extract_uri_parameters to fill in the placeholders with the provided args and kwargs. This ensures that resolve() returns a concrete URI string without unresolved placeholders.
232-238: 🛠️ Refactor suggestion
Path/Template mismatch silently degrades to empty parameter set.
If the incoming URI does not match the template (typo, missing segment, etc.), no parameters are extracted and the function is invoked without arguments.
This results in a genericResourceException, making it hard to diagnose the root cause.Recommend checking that every required template parameter was extracted and raising a clear
ValueError(400-style) before calling the underlying coroutine.params = self._extract_uri_parameters(uri) missing = self._extract_template_parameters(self.declaration.uri_template) - params.keys() if missing: raise ValueError(f"URI '{uri}' does not satisfy template parameters: {', '.join(sorted(missing))}")Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/draive/resources/template.py around lines 232 to 238, the code silently proceeds with an empty parameter set if the incoming URI does not match the template, causing a generic ResourceException later. To fix this, after extracting parameters, add a check to ensure all required template parameters are present. If any required parameter is missing, raise a clear ValueError with a 400-style error before invoking the underlying coroutine, improving error clarity and early failure detection.
49-56: 🛠️ Refactor suggestion
RFC 6570 operator coverage is incomplete in
_extract_template_parameters.Currently only the
/,#, and?operators are stripped.
RFC 6570 level-1/2 templates also include+,.,;, and&operators (and the reserved modifier+).
Templates such as{+path}or{;param}would therefore be reported back verbatim (+path,;param) and subsequently fail the function-parameter validation even when a matching argument exists.- if match.startswith("/") or match.startswith("#"): + if match[:1] in {"/", "#", "+", ".", ";", "&"}: params.add(match[1:])Consider expanding the set (or using
match.lstrip("/#+.;&")) so validation works for every legal operator.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/draive/resources/template.py around lines 49 to 56, the method _extract_template_parameters only strips the '/', '#', and '?' operators from URI template parameters, missing other RFC 6570 operators like '+', '.', ';', and '&'. To fix this, update the code to strip all valid RFC 6570 operators by expanding the set of characters removed from the start of the parameter strings, for example by using match.lstrip("/#+.;&"). This ensures all legal operators are handled and parameter validation works correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/draive/resources/template.py (1)
96-96: Consider implementing URI template resolution with actual parameters.The TODO comment indicates that the URI should be resolved with actual arguments rather than using the template. This would provide more accurate URI representation in the resolved resource.
Consider implementing URI template resolution:
- uri=self.declaration.uri_template, # TODO: resolve uri with args + uri=self._resolve_uri_template(*args, **kwargs),You would need to add a
_resolve_uri_templatemethod that substitutes the template parameters with actual values.guides/BasicStageUsage.md (1)
1-351: Excellent comprehensive documentation with minor grammar improvements needed.This guide provides outstanding coverage of Stage usage in draive, with clear explanations and practical examples. The structure flows logically from basic concepts to advanced patterns.
Consider addressing these minor grammar improvements for better readability:
- - Receives `StageState(context: LMMContext, result: MultimodalContent)` - - Performs processing or transformation + - Receives `StageState(context: LMMContext, result: MultimodalContent)` + - Performs processing or transformation-LMMOutput will be also used as the result of this stage. +LMMOutput will also be used as the result of this stage.-Make sure to have equal number of inputs and outputs in the result context. +Make sure to have an equal number of inputs and outputs in the result context.-Meta can be also altered when needed by making copies of stage. +Meta can also be altered when needed by making copies of the stage.-Stages can be also created fully customized by using the `stage` decorator: +Stages can also be created fully customized by using the `stage` decorator:🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - ReceivesStageState(context: LMMContext, result: MultimodalContent)- Performs processing or transformation - Returns ...(AGREEMENT_SENT_START)
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...(ALSO_PLACEMENT)
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...(BUNCH_OF)
[grammar] ~85-~85: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...ot change the result. Make sure to have equal number of inputs and outputs in the result contex...(BUNCH_OF)
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (10)
guides/BasicStageUsage.md(1 hunks)pyproject.toml(1 hunks)src/draive/__init__.py(2 hunks)src/draive/mcp/server.py(1 hunks)src/draive/parameters/__init__.py(2 hunks)src/draive/parameters/validation.py(4 hunks)src/draive/resources/__init__.py(2 hunks)src/draive/resources/template.py(5 hunks)src/draive/resources/types.py(2 hunks)tests/test_resource_uri_template.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- pyproject.toml
- src/draive/init.py
- src/draive/resources/init.py
- src/draive/parameters/init.py
- src/draive/resources/types.py
- src/draive/mcp/server.py
- tests/test_resource_uri_template.py
🧰 Additional context used
🪛 LanguageTool
guides/BasicStageUsage.md
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - Receives StageState(context: LMMContext, result: MultimodalContent) - Performs processing or transformation - Returns ...
(AGREEMENT_SENT_START)
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...
(ALSO_PLACEMENT)
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[grammar] ~85-~85: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...ot change the result. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
🔇 Additional comments (11)
src/draive/parameters/validation.py (4)
713-778: LGTM! Excellent refactoring with pattern matching.The refactored
_prepare_validator_of_floatfunction uses structural pattern matching effectively and provides flexible input handling by acceptingfloat,int, andstrtypes with proper conversion logic. The error handling for invalid string conversions is appropriate.
781-854: LGTM! Well-implemented integer validation with proper type checking.The new
_prepare_validator_of_intfunction correctly handles:
- Direct
intvaluesfloatvalues (with validation that the float equals its integer cast)strvalues with proper parsing and error handlingThe logic for float-to-int conversion (
value == validated) ensures that only whole number floats are accepted, which is the correct behavior.
856-927: LGTM! Comprehensive boolean validation with flexible string parsing.The new
_prepare_validator_of_boolfunction provides excellent flexibility by accepting:
- Direct
boolvaluesintvalues (nonzero check:bool(value != 0))strvalues with case-insensitive parsing for common boolean representationsThe string parsing logic covers standard boolean representations ("true"/"false", "t"/"f", "1"/"0") which enhances usability.
1238-1238: LGTM! Correct mapping update.The VALIDATORS mapping is correctly updated to use the new
_prepare_validator_of_intfunction instead of the generic_prepare_validator_of_type, which enables the enhanced integer validation logic.src/draive/resources/template.py (7)
1-4: LGTM! Necessary imports for URI template functionality.The added imports (
re,Callable,Coroutine,Mapping,Sequence,Set,urlparsecomponents) are all needed for the new URI template parsing and resolution functionality.
49-56: LGTM! Excellent parameter validation logic.The validation ensures that all URI template parameters exist as function arguments, preventing runtime errors. This is a crucial safety check for the template system.
104-124: LGTM! Comprehensive URI template parameter extraction.The
_extract_template_parametersmethod correctly handles various RFC 6570 URI template patterns:
- Basic
{param}patterns- Path expansion
{/param}patterns- Query parameters
{?var1,var2}patterns- Fragment
{#param}patternsThe regex pattern and parameter cleaning logic is sound.
126-152: LGTM! Proper query and fragment parameter extraction.Both
_extract_query_parametersand_extract_fragment_parametersmethods handle URL parsing correctly:
- Query parameters use
parse_qsand take the first value when multiple exist- Fragment parameters check for template patterns before extraction
154-198: LGTM! Robust path parameter extraction with regex matching.The
_extract_path_templateand_extract_path_parameters_with_slashmethods handle complex URI template patterns effectively. The regex construction and matching logic properly handles both{/param}and{param}patterns in paths.
199-240: LGTM! Clean simple path parameter extraction.The
_extract_simple_path_parametersand_extract_uri_parametersmethods provide a comprehensive solution for extracting parameters from URIs. The combination of different extraction methods covers all RFC 6570 URI template scenarios.
247-266: LGTM! Smart URI resolution with parameter extraction.The updated
resolve_from_urimethod intelligently:
- Extracts parameters from the provided URI using the template
- Calls the function with extracted parameters if available
- Falls back to parameterless call otherwise
- Uses the actual URI (not template) in the result
This provides excellent flexibility for resource resolution.
KaQuMiQ
force-pushed
the
feature/resource_uri
branch
from
431ac39 to
d603e89
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
guides/BasicStageUsage.md (5)
11-11: Fix grammar issue in the bulleted list.The static analysis correctly identifies a grammar agreement error. The list structure needs consistency.
- Receives `StageState(context: LMMContext, result: MultimodalContent)` - Performs processing or transformation - Returns updated `StageState` + Receives `StageState(context: LMMContext, result: MultimodalContent)` + Performs processing or transformation + Returns updated `StageState`🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - ReceivesStageState(context: LMMContext, result: MultimodalContent)- Performs processing or transformation - Returns ...(AGREEMENT_SENT_START)
51-51: Improve adverb placement for better flow.Move "also" before the verb for more natural reading.
-LMMOutput will be also used as the result of this stage. +LMMOutput will also be used as the result of this stage.🧰 Tools
🪛 LanguageTool
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...(ALSO_PLACEMENT)
65-65: Add missing articles for grammatical correctness.Both instances need the article "an" for proper grammar.
-Make sure to have equal number of inputs and outputs in the result context. +Make sure to have an equal number of inputs and outputs in the result context.Apply the same fix to line 85.
Also applies to: 85-85
🧰 Tools
🪛 LanguageTool
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...(BUNCH_OF)
158-158: Fix word order and add missing article.The sentence structure needs improvement for clarity.
-Meta can be also altered when needed by making copies of stage. +Meta can also be altered when needed by making copies of the stage.🧰 Tools
🪛 LanguageTool
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
278-278: Improve word order for better readability.Restructure the sentence for clearer flow.
-Stages can be also created fully customized by using the `stage` decorator: +Stages can also be created fully customized by using the `stage` decorator:🧰 Tools
🪛 LanguageTool
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
src/draive/resources/template.py (1)
177-205: Complex path parameter extraction needs refactoring.This method handles path parameters with slash prefixes but has high complexity. Consider breaking it down into smaller, more focused methods for better maintainability.
The logic is correct but could benefit from decomposition:
def _extract_path_parameters_with_slash(self, actual_path: str) -> dict[str, str]: params: dict[str, str] = {} slash_params = self._get_slash_parameters() template_for_regex = self._build_regex_template(slash_params) return self._match_and_extract_params(template_for_regex, actual_path, slash_params) def _get_slash_parameters(self) -> list[str]: return re.findall(r"\{/([^}]+)\}", self.declaration.uri_template) def _build_regex_template(self, slash_params: list[str]) -> str: # Extract the regex building logic pass def _match_and_extract_params(self, template: str, path: str, params: list[str]) -> dict[str, str]: # Extract the matching and parameter extraction logic pass
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (10)
guides/BasicStageUsage.md(1 hunks)pyproject.toml(1 hunks)src/draive/__init__.py(2 hunks)src/draive/mcp/server.py(3 hunks)src/draive/parameters/__init__.py(2 hunks)src/draive/parameters/validation.py(4 hunks)src/draive/resources/__init__.py(2 hunks)src/draive/resources/template.py(6 hunks)src/draive/resources/types.py(2 hunks)tests/test_resource_uri_template.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/draive/resources/init.py
- pyproject.toml
- src/draive/parameters/init.py
- src/draive/init.py
- src/draive/resources/types.py
- src/draive/mcp/server.py
- tests/test_resource_uri_template.py
🧰 Additional context used
🪛 LanguageTool
guides/BasicStageUsage.md
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - Receives StageState(context: LMMContext, result: MultimodalContent) - Performs processing or transformation - Returns ...
(AGREEMENT_SENT_START)
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...
(ALSO_PLACEMENT)
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[grammar] ~85-~85: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...ot change the result. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
🔇 Additional comments (11)
guides/BasicStageUsage.md (2)
22-49: Comprehensive code examples demonstrate good practices.The completion stage examples show proper usage patterns including basic completion, tool integration, and structured output formatting. The examples are syntactically correct and follow expected framework patterns.
304-348: Excellent comprehensive example showcasing framework capabilities.This complete example effectively demonstrates multiple Stage concepts including:
- Sequential composition with
Stage.sequence()- Conditional execution with
.when()- Method chaining with
.cached(),.with_retry(), and.traced()- Proper async context management with
ctx.scope()- Tool integration and error handling
The code structure follows best practices and provides a realistic use case.
src/draive/parameters/validation.py (4)
713-778: Well-implemented float validation with pattern matching.The refactored
_prepare_validator_of_floatfunction properly uses structural pattern matching and handles multiple input types:
- Direct float values
- Integer conversion to float
- String parsing with proper error handling
The logic is sound and the error handling is appropriate.
781-854: Robust integer validation with precise float handling.The new
_prepare_validator_of_intfunction correctly:
- Accepts native integers directly
- Validates float-to-int conversion by checking equality (
value == validated)- Parses strings with proper exception handling
- Maintains type safety throughout
The float equality check prevents precision loss issues.
856-928: Comprehensive boolean validation with flexible string parsing.The
_prepare_validator_of_boolfunction provides excellent flexibility:
- Direct boolean handling
- Integer-to-boolean conversion using truthiness
- String parsing supporting multiple formats ("true"/"t"/"1" and "false"/"f"/"0")
- Case-insensitive string matching
This covers common boolean representation formats effectively.
1237-1238: Proper integration of new validators into the mapping.The VALIDATORS mapping correctly associates the new validator functions with their respective types. The placement maintains logical organization of the mapping.
src/draive/resources/template.py (5)
50-58: Excellent validation of URI template parameters.The constructor properly validates that all URI template parameters exist as function arguments, preventing runtime errors. This early validation ensures template consistency and provides clear error messages.
112-132: Comprehensive RFC 6570 parameter extraction.The
_extract_template_parametersmethod correctly handles various RFC 6570 patterns:
- Basic variables
{var}- Path variables
{/var}- Fragment variables
{#var}- Query parameters
{?var1,var2}The regex pattern and parameter cleanup logic is sound.
227-247: Well-structured URI parameter extraction orchestration.The
_extract_uri_parametersmethod effectively coordinates different parameter extraction strategies based on URI template patterns. The conditional logic for handling slash-prefixed parameters vs. simple path parameters is appropriate.
249-261: Robust URI matching with proper error handling.The
matches_urimethod correctly handles both simple string matching (when no parameters) and regex matching (for templated URIs). The regex error handling prevents crashes from malformed patterns.
269-278: Flexible parameter-based resource resolution.The conditional logic in
resolve_from_uriproperly handles both parameterized and non-parameterized resource calls. The parameter extraction and function invocation logic is sound.
KaQuMiQ
force-pushed
the
feature/resource_uri
branch
from
d603e89 to
6a4e18e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
guides/BasicStageUsage.md (5)
11-11: Fix grammar issue in bullet point list.The sentence structure is awkward. The bullet points should maintain parallel structure.
-- Receives `StageState(context: LMMContext, result: MultimodalContent)` -- Performs processing or transformation +- Receives `StageState(context: LMMContext, result: MultimodalContent)` +- Performs processing or transformation🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - ReceivesStageState(context: LMMContext, result: MultimodalContent)- Performs processing or transformation - Returns ...(AGREEMENT_SENT_START)
51-51: Improve word order for better flow.The word placement can be improved for better readability.
-Each completion stage will extend the context with input, generated output, and tools used. LMMOutput will be also used as the result of this stage. +Each completion stage will extend the context with input, generated output, and tools used. LMMOutput will also be used as the result of this stage.🧰 Tools
🪛 LanguageTool
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...(ALSO_PLACEMENT)
65-65: Add missing articles for better grammar.The phrase needs articles for proper grammar.
-Make sure to have equal number of inputs and outputs in the result context. +Make sure to have an equal number of inputs and outputs in the result context.Apply this change to both occurrences (lines 65 and 85).
Also applies to: 85-85
🧰 Tools
🪛 LanguageTool
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...(BUNCH_OF)
158-158: Improve word order and add missing article.The sentence structure can be improved for clarity.
-Meta can be also altered when needed by making copies of stage. +Meta can also be altered when needed by making copies of the stage.🧰 Tools
🪛 LanguageTool
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
278-278: Improve word order for better flow.Reorder words for more natural reading.
-Stages can be also created fully customized by using the `stage` decorator: +Stages can also be created fully customized by using the `stage` decorator:🧰 Tools
🪛 LanguageTool
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (10)
guides/BasicStageUsage.md(1 hunks)pyproject.toml(1 hunks)src/draive/__init__.py(2 hunks)src/draive/mcp/server.py(3 hunks)src/draive/parameters/__init__.py(2 hunks)src/draive/parameters/validation.py(4 hunks)src/draive/resources/__init__.py(2 hunks)src/draive/resources/template.py(6 hunks)src/draive/resources/types.py(2 hunks)tests/test_resource_uri_template.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- pyproject.toml
- src/draive/resources/init.py
- src/draive/parameters/init.py
- src/draive/init.py
- src/draive/mcp/server.py
- src/draive/resources/types.py
- src/draive/resources/template.py
🧰 Additional context used
🪛 LanguageTool
guides/BasicStageUsage.md
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - Receives StageState(context: LMMContext, result: MultimodalContent) - Performs processing or transformation - Returns ...
(AGREEMENT_SENT_START)
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...
(ALSO_PLACEMENT)
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[grammar] ~85-~85: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...ot change the result. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
🔇 Additional comments (12)
src/draive/parameters/validation.py (4)
713-779: LGTM! Improved pattern matching approach with comprehensive input handling.The refactored
_prepare_validator_of_floatfunction correctly implements pattern matching to handle multiple input types (float, int, str) with proper validation and conversion logic. The error handling for invalid string conversions is appropriate.
781-854: LGTM! Well-structured integer validation with float conversion logic.The new
_prepare_validator_of_intfunction properly handles integer validation including:
- Direct int values
- Float values that equal their integer conversion (e.g., 5.0 → 5)
- String parsing with proper exception handling
The float-to-int conversion with equality check ensures only valid integer-equivalent floats are accepted.
856-928: LGTM! Comprehensive boolean validation with flexible string parsing.The new
_prepare_validator_of_boolfunction provides excellent flexibility for boolean validation:
- Direct bool values
- Integer conversion (non-zero → True, zero → False)
- String parsing with multiple accepted formats ("true"/"t"/"1" for True, "false"/"f"/"0" for False)
The case-insensitive string matching using
.lower()is a good UX improvement.
1237-1238: LGTM! Correct mapping updates for specialized validators.The VALIDATORS mapping correctly associates
boolwith the new specialized_prepare_validator_of_boolandintwith_prepare_validator_of_int, enabling the enhanced validation logic.guides/BasicStageUsage.md (1)
1-351: Overall excellent documentation structure and content.This guide provides comprehensive coverage of Stage concepts and usage patterns. The progression from basic concepts to advanced patterns is logical and well-organized. The code examples effectively demonstrate the API usage.
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - ReceivesStageState(context: LMMContext, result: MultimodalContent)- Performs processing or transformation - Returns ...(AGREEMENT_SENT_START)
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...(ALSO_PLACEMENT)
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...(BUNCH_OF)
[grammar] ~85-~85: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...ot change the result. Make sure to have equal number of inputs and outputs in the result contex...(BUNCH_OF)
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
tests/test_resource_uri_template.py (7)
8-19: LGTM! Clean test for basic URI template parameter extraction.The test correctly validates simple variable expansion from URI templates, with appropriate assertions for the extracted parameters.
21-34: LGTM! Good coverage of query parameter extraction.The test properly validates query parameter extraction using RFC 6570 query expansion syntax (
{?q,limit}). The assertions correctly check for parameter presence.
35-48: LGTM! Comprehensive test for multiple path parameters.Excellent test coverage for complex path parameter extraction with multiple variables. The test data and assertions are appropriate.
80-91: LGTM! Good async test for resource resolution with parameters.The test properly validates the
resolve_from_urimethod with parameter extraction and resource result structure.
188-204: LGTM! Excellent validation test for template parameter checking.This test properly validates that the resource decorator correctly identifies when URI template parameters don't match function parameters, ensuring proper error handling.
1-213: Comprehensive test coverage with room for improvement.The test suite provides excellent coverage of URI template functionality including parameter extraction, resource resolution, and validation. The tests are well-structured and use appropriate async/sync patterns.
149-160:Details
❌ Incorrect review comment
Verify path segment expansion test logic.
The test uses path segment expansion syntax
{/resource}but the URI doesn't seem to match this pattern correctly. The template suggests/resourceshould be optional, but the test URI has/users.
🏁 Script executed:
#!/bin/bash # Check the URI template implementation for path segment expansion ast-grep --pattern '_extract_uri_parameters($_, $_) { $$$ }' # Look for RFC 6570 URI template parsing logic rg -A 10 -B 5 "path.*segment.*expansion|/.*\{.*\}"Length of output: 19433
🏁 Script executed:
#!/bin/bash # Show context around slash_params definition rg -n "slash_params" -A3 -B3 src/draive/resources/template.pyLength of output: 982
🏁 Script executed:
#!/bin/bash # Retrieve the full _extract_uri_parameters method rg -n "def _extract_uri_parameters" -A30 -B5 src/draive/resources/template.py # Retrieve the _extract_path_template helper rg -n "def _extract_path_template" -A30 -B5 src/draive/resources/template.pyLength of output: 2883
Path segment expansion is correct—no change needed.
The
{ /resource }operator is implemented in_extract_path_parameters_with_slashby replacing{/resource}with/([^/]+)and then matching/users/123against^/([^/]+)/([^/]+)$, yielding{"resource": "users", "resource_id": "123"}as expected. The test accurately verifies this behavior.Likely an incorrect or invalid review comment.
KaQuMiQ
force-pushed
the
feature/resource_uri
branch
from
6a4e18e to
9bcee0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
guides/BasicStageUsage.md (1)
346-347:⚠️ Potential issueComplete the example with proper argument.
The function call still uses a placeholder value and keyword argument syntax instead of the expected positional argument.
Based on the past review feedback and function signature, apply this fix:
-# Run the example -summary = await process_document(document="...") +# Run the example +sample_doc = "This is a sample document for processing and analysis." +summary = await process_document(sample_doc)
🧹 Nitpick comments (1)
guides/BasicStageUsage.md (1)
11-11: Address grammar and style issues for better readability.Several grammar and style improvements would enhance the documentation quality:
Consider these improvements:
- - Receives `StageState(context: LMMContext, result: MultimodalContent)` - - Performs processing or transformation + - Receive `StageState(context: LMMContext, result: MultimodalContent)` + - Perform processing or transformation -LMMOutput will be also used as the result of this stage. +LMMOutput will also be used as the result of this stage. -Make sure to have equal number of inputs and outputs +Make sure to have an equal number of inputs and outputs -Meta can be also altered when needed by making copies of stage. +Meta can also be altered when needed by making copies of the stage. -Stages can be also created fully customized by using the `stage` decorator: +Stages can also be created fully customized by using the `stage` decorator:Also applies to: 51-51, 65-65, 85-85, 158-158, 278-278
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - ReceivesStageState(context: LMMContext, result: MultimodalContent)- Performs processing or transformation - Returns ...(AGREEMENT_SENT_START)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (10)
guides/BasicStageUsage.md(1 hunks)pyproject.toml(1 hunks)src/draive/__init__.py(2 hunks)src/draive/mcp/server.py(3 hunks)src/draive/parameters/__init__.py(2 hunks)src/draive/parameters/validation.py(4 hunks)src/draive/resources/__init__.py(2 hunks)src/draive/resources/template.py(6 hunks)src/draive/resources/types.py(2 hunks)tests/test_resource_uri_template.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- src/draive/resources/init.py
- src/draive/parameters/init.py
- src/draive/init.py
- src/draive/resources/types.py
- tests/test_resource_uri_template.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/draive/mcp/server.py (2)
src/draive/resources/template.py (4)
ResourceTemplate(27-288)resource(315-340)resolve_from_uri(263-288)matches_uri(249-261)src/draive/resources/types.py (1)
Resource(137-243)
🪛 LanguageTool
guides/BasicStageUsage.md
[grammar] ~11-~11: Possible agreement error. You should probably use: “Receive Performs” or “Receives perform”.
Context: ...s within an LMM context. Each stage: - Receives StageState(context: LMMContext, result: MultimodalContent) - Performs processing or transformation - Returns ...
(AGREEMENT_SENT_START)
[style] ~51-~51: To make your writing flow more naturally, try moving ‘also’ before the verb.
Context: ...rated output, and tools used. LMMOutput will be also used as the result of this stage. ### ...
(ALSO_PLACEMENT)
[grammar] ~65-~65: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...n the result context. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[grammar] ~85-~85: It seems that an article is missing. Did you mean “the equal number of” or “a equal number of”?
Context: ...ot change the result. Make sure to have equal number of inputs and outputs in the result contex...
(BUNCH_OF)
[uncategorized] ~158-~158: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ...cessing", "nlp"), } ) ``` Meta can be also altered when needed by making copies of...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~158-~158: You might be missing the article “the” here.
Context: ...altered when needed by making copies of stage. ```python documented_stage = Stage.co...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~278-~278: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: ... ### Custom Stage Functions Stages can be also created fully customized by using the `...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
🔇 Additional comments (4)
src/draive/mcp/server.py (1)
147-192: Well-structured separation of templated and non-templated resources.The logic correctly distinguishes between
ResourceTemplateinstances with arguments (requiring template matching) and those without arguments (treated as regular resources). The consistent use ofuri_templatethroughout is appropriate for the new URI template support.src/draive/parameters/validation.py (1)
713-927: Excellent refactoring of primitive type validators.The changes significantly improve the parameter validation system:
- Pattern matching approach: Cleaner and more maintainable than isinstance checks
- Enhanced input flexibility: Float accepts int/str, int accepts float (when equivalent), bool accepts int/str with intelligent parsing
- Better separation of concerns: Each primitive type now has its own dedicated validator
The string parsing for booleans is particularly useful for handling user input with values like "true", "false", "1", "0".
src/draive/resources/template.py (2)
36-86: Robust URI template parameter validation.The constructor properly validates that all URI template parameters have corresponding function arguments, preventing runtime errors. The preparation of match patterns only when needed (when parameters exist) is an efficient optimization.
249-288: Well-implemented URI matching and resolution.The
matches_uriandresolve_from_urimethods provide clean interfaces for the template functionality. The fallback to exact string comparison when no pattern matching is needed is appropriate, and the parameter extraction with dynamic function calling is well-handled.
| for template in available_resource_templates: | ||
| if not template.matches_uri(uri_string): | ||
| resource = await template.resolve_from_uri(uri_string) | ||
| break |
There was a problem hiding this comment.
Fix critical logic error in template matching loop.
The condition is inverted - the code resolves resources when templates do NOT match, which will never work correctly.
Apply this fix:
- for template in available_resource_templates:
- if not template.matches_uri(uri_string):
- resource = await template.resolve_from_uri(uri_string)
- break
+ for template in available_resource_templates:
+ if template.matches_uri(uri_string):
+ resource = await template.resolve_from_uri(uri_string)
+ break📝 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.
| for template in available_resource_templates: | |
| if not template.matches_uri(uri_string): | |
| resource = await template.resolve_from_uri(uri_string) | |
| break | |
| for template in available_resource_templates: | |
| if template.matches_uri(uri_string): | |
| resource = await template.resolve_from_uri(uri_string) | |
| break |
🤖 Prompt for AI Agents
In src/draive/mcp/server.py around lines 232 to 235, the logic in the loop is
inverted; it currently resolves the resource when the template does NOT match
the URI, which is incorrect. Change the condition to check if the template
matches the URI (remove the negation) before resolving the resource. This
ensures resource resolution only happens for matching templates.
| def _extract_template_parameters( | ||
| self, | ||
| uri: str, | ||
| ) -> Set[str]: | ||
| params: set[str] = set() | ||
| # Find all {param} patterns in the URI template | ||
| for match in re.findall(r"\{([^}]+)\}", uri): | ||
| # Clean up parameter names (remove RFC 6570 prefixes) | ||
| # Remove RFC 6570 prefixes | ||
| if match.startswith("/") or match.startswith("#"): | ||
| params.add(match[1:]) | ||
|
|
||
| elif match.startswith("?"): | ||
| # Query parameters can be comma-separated: {?var1,var2} | ||
| for p in match[1:].split(","): | ||
| params.add(p.strip()) | ||
|
|
||
| else: | ||
| params.add(match) | ||
|
|
||
| return params | ||
|
|
||
| def _extract_query_parameters( | ||
| self, | ||
| parsed_uri: ParseResult, | ||
| ) -> dict[str, str]: | ||
| params: dict[str, str] = {} | ||
| if parsed_uri.query: | ||
| query_params = parse_qs(parsed_uri.query) | ||
| for key, values in query_params.items(): | ||
| if values: | ||
| params[key] = values[0] # Take first value | ||
| return params | ||
|
|
||
| def _extract_fragment_parameters( | ||
| self, | ||
| parsed_uri: ParseResult, | ||
| ) -> dict[str, str]: | ||
| params: dict[str, str] = {} | ||
| if parsed_uri.fragment and "{#" in self.declaration.uri_template: | ||
| # Find fragment template pattern {#var} | ||
| fragment_start = self.declaration.uri_template.find("{#") | ||
| if fragment_start != -1: | ||
| fragment_end = self.declaration.uri_template.find("}", fragment_start) | ||
| if fragment_end != -1: | ||
| param_name = self.declaration.uri_template[fragment_start + 2 : fragment_end] | ||
| params[param_name] = parsed_uri.fragment | ||
|
|
||
| return params | ||
|
|
||
| def _extract_path_template(self) -> str: | ||
| if "://" in self.declaration.uri_template: | ||
| remainder = self.declaration.uri_template.split("://", 1)[1] | ||
| # Find where the netloc ends (first occurrence of {, /, ?, or #) | ||
| netloc_end = len(remainder) | ||
| for char in ["/", "{", "?", "#"]: | ||
| pos = remainder.find(char) | ||
| if pos != -1 and pos < netloc_end: | ||
| netloc_end = pos | ||
|
|
||
| return remainder[netloc_end:] | ||
|
|
||
| else: | ||
| return self.declaration.uri_template | ||
|
|
||
| def _extract_path_parameters_with_slash( | ||
| self, | ||
| actual_path: str, | ||
| ) -> dict[str, str]: | ||
| params: dict[str, str] = {} | ||
| slash_params: list[Any] = re.findall(r"\{/([^}]+)\}", self.declaration.uri_template) | ||
| path_template: str = self._extract_path_template() | ||
|
|
||
| # Create regex pattern | ||
| template_for_regex: str = path_template | ||
| for param in slash_params: | ||
| template_for_regex = template_for_regex.replace(f"{{/{param}}}", "/([^/]+)") | ||
|
|
||
| # Handle regular {var} patterns | ||
| regular_params: list[Any] = re.findall(r"\{([^}]+)\}", template_for_regex) | ||
| for param in regular_params: | ||
| if not param.startswith("/"): # Skip already handled slash patterns | ||
| template_for_regex = template_for_regex.replace(f"{{{param}}}", "([^/]+)") | ||
|
|
||
| # Match against the actual path | ||
| match: re.Match[str] | None = re.match(f"^{template_for_regex}$", actual_path) | ||
| if match: | ||
| all_params: list[Any] = slash_params + [ | ||
| p for p in regular_params if not p.startswith("/") | ||
| ] | ||
| for param, value in zip(all_params, match.groups(), strict=False): | ||
| params[param] = value | ||
|
|
||
| return params | ||
|
|
||
| def _extract_simple_path_parameters( | ||
| self, | ||
| parsed_template: ParseResult, | ||
| actual_path: str, | ||
| ) -> dict[str, str]: | ||
| params: dict[str, str] = {} | ||
| template_parts = parsed_template.path.split("/") | ||
| actual_parts = actual_path.split("/") | ||
|
|
||
| for template_part, actual_part in zip(template_parts, actual_parts, strict=False): | ||
| if template_part.startswith("{") and template_part.endswith("}"): | ||
| param_name = template_part[1:-1] # Remove braces | ||
| # Skip query and fragment patterns | ||
| if param_name.startswith("?") or param_name.startswith("#"): | ||
| continue | ||
|
|
||
| params[param_name] = actual_part | ||
|
|
||
| return params | ||
|
|
||
| def _extract_uri_parameters( | ||
| self, | ||
| uri: str, | ||
| /, | ||
| ) -> Mapping[str, str]: | ||
| params: dict[str, str] = {} | ||
| parsed_uri: ParseResult = urlparse(uri) | ||
| parsed_template: ParseResult = urlparse(self.declaration.uri_template) | ||
|
|
||
| # Extract different types of parameters | ||
| params.update(self._extract_query_parameters(parsed_uri)) | ||
| params.update(self._extract_fragment_parameters(parsed_uri)) | ||
|
|
||
| # Handle path parameters | ||
| if "{/" in self.declaration.uri_template: | ||
| params.update(self._extract_path_parameters_with_slash(parsed_uri.path)) | ||
|
|
||
| else: | ||
| params.update(self._extract_simple_path_parameters(parsed_template, parsed_uri.path)) | ||
|
|
||
| return params | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Comprehensive URI parameter extraction implementation.
The parameter extraction logic correctly handles the different RFC 6570 patterns:
- Query parameters (
{?var}) - Fragment parameters (
{#var}) - Path parameters (
{var}and{/var})
The implementation accounts for the complexity of path parameter extraction with both simple and slash-prefixed patterns.
Given the complexity of this URI parsing logic, please verify it works correctly with comprehensive test cases:
🏁 Script executed:
#!/bin/bash
# Verify the URI template parameter extraction works correctly
rg -A 10 "test.*extract.*parameter" tests/
rg -A 10 "test.*uri.*template" tests/Length of output: 1014
Additional URI extraction test cases needed
The existing tests in tests/test_resource_uri_template.py only cover simple path templates (e.g. /{org}/{repo}/{issue_id}) via test_resolve_from_uri_complex_template. There are no tests for:
- Query parameters (
{?var1,var2}) - Fragment parameters (
{#var}) - Slash-prefixed path parameters (
{/var})
Please add tests to ensure:
_extract_query_parameterscorrectly parses single and comma-separated query templates._extract_fragment_parametershandles fragment templates only when{#…}is present._extract_path_parameters_with_slashmatches/valuesegments for{/{var}}.
Example snippets to add to tests/test_resource_uri_template.py:
import pytest
from draive.resources.template import _extract_uri_parameters # or through .resolve_from_uri
@pytest.mark.parametrize("uri, template, expected", [
(
"https://api.example.com/items?foo=1&bar=2",
"https://api.example.com/items{?foo,bar}",
{"foo": "1", "bar": "2"}
),
(
"https://api.example.com/items#token123",
"https://api.example.com/items{#token}",
{"token": "token123"}
),
(
"https://api.example.com/items/42/detail",
"https://api.example.com/items{/id}/detail",
{"id": "42"}
),
])
def test_extract_misc_parameters(uri, template, expected):
class DummyDecl: uri_template = template
tpl = TemplateExtractor(declaration=DummyDecl()) # or the actual class wrapping methods
result = tpl._extract_uri_parameters(uri)
assert result == expectedWith these in place, we can confirm coverage for all RFC 6570 patterns.
🤖 Prompt for AI Agents
In src/draive/resources/template.py from lines 112 to 248, the current tests
only cover simple path templates and lack coverage for query parameters,
fragment parameters, and slash-prefixed path parameters extraction. To fix this,
add new test cases in tests/test_resource_uri_template.py that verify
_extract_query_parameters correctly parses query templates with single and
multiple variables, _extract_fragment_parameters handles fragment templates only
when {#…} is present, and _extract_path_parameters_with_slash matches segments
for {/var} patterns. Use parameterized tests with example URIs, templates, and
expected parameter dictionaries to ensure comprehensive coverage of all RFC 6570
parameter extraction scenarios.
1 participant
No description provided.