-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ResponseRagStage
and PromptResponseRagModule
updates
#1056
Conversation
# Conflicts: # docs/examples/query-webpage-astra-db.md # docs/examples/talk-to-a-pdf.md # docs/examples/talk-to-a-webpage.md # docs/griptape-framework/engines/rag-engines.md # docs/griptape-framework/structures/task-memory.md # docs/griptape-framework/structures/tasks.md # docs/griptape-tools/official-tools/rag-client.md
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
CHANGELOG.md
Outdated
**BREAKING**: Removed before and after response modules from `ResponseRagStage`. | ||
**BREAKING**: Moved ruleset and metadata ingestion from standalone modules to `PromptResponseRagModule`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing hypens
prompt_driver: BasePromptDriver = field() | ||
answer_token_offset: int = field(default=400) | ||
rulesets: list[Ruleset] = field(factory=list) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should implement RuleMixin
instead of defining itself.
params: dict[str, Any] = {"text_chunks": [c.to_text() for c in artifacts]} | ||
|
||
if len(self.rulesets) > 0: | ||
params["rulesets"] = J2("rulesets/rulesets.j2").render(rulesets=self.rulesets) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After implementing RuleMixin
, replace self.rulesets
with self.all_rulesets
.
artifacts = WebLoader(max_tokens=500).load("https://www.griptape.ai") | ||
if isinstance(artifacts, ErrorArtifact): | ||
raise ValueError(artifacts.value) | ||
|
||
vector_store.upsert_text_artifacts({"griptape": artifacts}) | ||
vector_store.upsert_text_artifacts( | ||
{ | ||
"griptape": WebLoader(max_tokens=500).load("https://www.griptape.ai"), | ||
} | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this pass type checks?
Edit: it probably will not: #1057
name: str = field( | ||
default=Factory(lambda self: f"{self.__class__.__name__}-{uuid.uuid4().hex}", takes_self=True), kw_only=True | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does name uniqueness get us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much easier to add multiple modules of the same type without having to explicitly define names. May be that's something we add to tools as well?
def modules(self) -> list[BaseRagModule]: | ||
ms = [] | ||
|
||
ms.extend(self.before_response_modules) | ||
ms.extend(self.after_response_modules) | ||
|
||
if self.response_module is not None: | ||
ms.append(self.response_module) | ||
ms.extend(self.response_modules) | ||
|
||
return ms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do because we test for module name uniqueness by using this property.
Describe your changes
RagContext.output
was changed toRagContext.outputs
to support multiple outputs. All relevant RAG modules we adjusted accordingly.ResponseRagStage
.PromptResponseRagModule
.RagEngine
modules.📚 Documentation preview 📚: https://griptape--1056.org.readthedocs.build//1056/