Skip to content
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

Add Structured Output #1443

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Structured Output #1443

wants to merge 1 commit into from

Conversation

collindutter
Copy link
Member

@collindutter collindutter commented Dec 13, 2024

Describe your changes

Issue ticket number and link

Closes #1468
Closes #1467

Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@collindutter collindutter force-pushed the feature/structured-output branch 3 times, most recently from a7ed072 to f74e2e6 Compare December 18, 2024 23:56
@collindutter collindutter changed the title Add StructuredOutputTool Add Structured Output Dec 19, 2024
@collindutter collindutter added the type: feature request New feature or request label Dec 19, 2024
@collindutter collindutter force-pushed the feature/structured-output branch 14 times, most recently from e96721f to d2c0827 Compare December 24, 2024 18:19
@collindutter collindutter force-pushed the feature/structured-output branch from d2c0827 to d07d699 Compare December 24, 2024 18:54
@collindutter collindutter marked this pull request as ready for review December 24, 2024 19:05
@collindutter
Copy link
Member Author

@dylanholmes @vachillo still working through docs and some final cleanup, but the PR is certainly developed/large enough for first pass of reviews.

Comment on lines +300 to +318
def _add_native_schema_to_prompt_stack(self, stack: PromptStack, rulesets: list[Ruleset]) -> None:
# Need to separate JsonSchemaRules from other rules, removing them in the process
json_schema_rules = [rule for ruleset in rulesets for rule in ruleset.rules if isinstance(rule, JsonSchemaRule)]
non_json_schema_rules = [
[rule for rule in ruleset.rules if not isinstance(rule, JsonSchemaRule)] for ruleset in rulesets
]
for ruleset, non_json_rules in zip(rulesets, non_json_schema_rules):
ruleset.rules = non_json_rules

schemas = [rule.value for rule in json_schema_rules if isinstance(rule.value, Schema)]

if len(json_schema_rules) != len(schemas):
warnings.warn(
"Not all provided `JsonSchemaRule`s include a `schema.Schema` instance. These will be ignored with `use_native_structured_output`.",
stacklevel=2,
)

if schemas:
stack.output_schema = schemas[0] if len(schemas) == 1 else Schema(Or(*schemas))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really don't like this method. Very open to suggestions on improvements.

Comment on lines -95 to +107
system_template = self.generate_system_template(self)
if system_template:
stack.add_system_message(system_template)
rulesets = self.rulesets
system_artifacts = [TextArtifact(self.generate_system_template(self))]
if self.prompt_driver.use_native_structured_output:
self._add_native_schema_to_prompt_stack(stack, rulesets)

# Ensure there is at least one Ruleset that has non-empty `rules`.
if any(len(ruleset.rules) for ruleset in rulesets):
system_artifacts.append(TextArtifact(J2("rulesets/rulesets.j2").render(rulesets=rulesets)))

# Ensure there is at least one system Artifact that has a non-empty value.
has_system_artifacts = any(system_artifact.value for system_artifact in system_artifacts)
if has_system_artifacts:
stack.add_system_message(ListArtifact(system_artifacts))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By separating out the rendering of Rules from generate_system_template it solves #1467 but only in the context of Prompt Task. There are plenty of other places where we use Rules (RagEngine) that would not benefit from this change.

Open to discussion on how we can solve this globally.

@collindutter collindutter self-assigned this Dec 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Structured Output Override System Prompts Without Losing Rulesets
1 participant