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

feat: added new trace types for Response step (PL3-54) #446

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

zhihil
Copy link
Contributor

@zhihil zhihil commented Jul 14, 2023

Fixes or implements PL3-54

Brief description. What is this change?

Added a new family of V3 traces to support the Response handler.

Implementation details. How do you make this change?

Adding new trace types to support new kinds of content in V3:

  1. JSONTrace - Stores JSON content
  2. VideoTrace - Stores Video content

Added new versions of existing traces to simplify the trace

  1. ImageTrace - Stores an image url without the additional properties from a VisualTrace. The v3 Response handler does not have sufficient data to fill out a VisualTrace. This ImageTrace is a simpler trace that avoids a situation where the Response handler returns a VisualTrace with extraneous, useless properties with dummy values.
  2. TextTrace - Stores text as Markup output. This replaces the V2 text trace, as the original text trace does not return Markup but instead, a string message along with Slate.js data. We should instead return a Markup data structure and leave it up to the user to translate Markup formatting (e.g. bolding) into a client-specific format (e.g. ** in markdown).
  3. DebugTrace - Refactors the debug trace to be more flexible. Highlights include a log-level property, DebugInfoLevel, to support a future feature where users can filter out for certain debug traces from general-runtime similar to different log levels in JS console logging and a new details payload that could store any data. In particular, I've used this in the Prompt variants to output debug traces containing the documents used by the knowledge base to generate its answer.

The new trace types are nested under traces/v3 not node. Ideally, in the future, traces are not coupled 1-to-1 with node. The traces should be a related, but essentially independent data type from the nodes and the general-runtime can be thought of as an "adapter" that adapts input nodes into output traces.

NOTE: This PR was originally submitted for review but has been resubmitted because it has undergone refactors as a result of the response handler changes.

Setup information

N/A

Deployment Notes

N/A

Related PRs

Checklist

  • Breaking changes have been communicated, including:
    • New required environment variables
    • Renaming of interfaces (API routes, request/response interface, etc)
  • New environment variables have been deployed
  • Appropriate tests have been written
    • Bug fixes are accompanied by an updated or new test
    • New features are accompanied by a new test

@zhihil zhihil self-assigned this Jul 14, 2023
@zhihil zhihil force-pushed the brennan/response-trace-types/PL3-54 branch from be23e96 to ed332f4 Compare July 14, 2023 19:56
Comment on lines +35 to +42
style?: {
delay?: number;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the idea to move this out of the text trace's payload?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. The new JSON variant seems to also have a sort of delay property, based on the Figma mocks.

@zhihil zhihil force-pushed the brennan/response-trace-types/PL3-54 branch from 591a9f6 to 6c50516 Compare July 19, 2023 19:08
@zhihil zhihil force-pushed the brennan/response-trace-types/PL3-54 branch 2 times, most recently from 8bf6576 to c772d1f Compare August 3, 2023 16:18
@zhihil zhihil force-pushed the brennan/response-trace-types/PL3-54 branch from c772d1f to 0baae44 Compare August 3, 2023 16:19
@sonarcloud
Copy link

sonarcloud bot commented Aug 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

warning The version of Java (11.0.3) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

import { BaseTraceFrame, TraceType } from '../../node/utils';

interface StepData {
content: Markup;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The data payload is subject to change due to a proposal by Tyler H to simply return Markdown rather than Markup. See slack thread here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants