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: implement response step handler (PL3-50) #551

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

Conversation

zhihil
Copy link
Contributor

@zhihil zhihil commented Jul 18, 2023

Fixes or implements PL3-50

Brief description. What is this change?

Implements the v3 response step handler. Functionality that is included with this PR includes:

  1. Text variants, JSON variants, prompt variants
  2. Support for all attachments in a Text variant
  3. A basic foundation for multichannel, multilingual discriminator logic
  4. A basic foundation for implementing conditions into the response handler

Implementation details. How do you make this change?

The response.handler.ts file is the best "entry point" for the code changes.

The response.handler.ts calls "framework" functions from:

  1. selectDiscriminator.ts
  2. evaluateVariant.ts
  3. evaluateCarousel.ts

These "framework" functions define the high-level logical flow of evaluating a v3 Response step. It includes hooks where we can register handlers for specific functionality like different Variants, Attachments, and Conditions.

The translation of Variants to traces is handled by the variant module. In particular:

  1. text.variant.ts - This handles translating content from a text variant into a V3 text trace
  2. json.variant.ts - This handles translating content from a JSON variant into a V3 JSON trace
  3. prompt.variant.ts - This generates a response using knowledge base or LLMs based on the prompt variant's configuration and outputs a V3 text trace containing the generated content.

The translation of Attachment CMS entities to traces is handled by the variant/attachment module. The individual handlers perform similar purposes as the variant handlers.

No Condition handlers are implemented at the moment, but the code has been designed to account for that functionality in the future. Skeleton files that can house a future implementation have already been written.

This PR also includes additional utility functions to support the implementation:

  1. variableContext - A reimplementation of the variables map and replaceVariables() function in the old V2 runtime. The main change is that (1) variable substitution is done on Markup data rather than regexing strings and (2) we package the data (variables) and the function (replaceVariables) into a single class VariableContext which is injected into specific handlers (e.g. Variant handlers). This was done to simplify code because replaceVariables()'s call signature already resembles a method call signature, i.e, why write replaceVariables(variables, ...) when you could just do variables.replaceVariables(...)?
  2. language-generator - This module reimplements some of the AI functionality in general-runtime to cleanup some messy patterns, e.g, in the V2 code, we insert a prompt with Voiceflow variables into a chat history and then perform a regex replace on all messages in the chat history (this work is useless because all but the first and last messages would actually have variables). The V3 language-generator module reimplements this does not perform any variable substitution whatsoever. The caller must resolve the variables before calling language-generator.
  3. markupUtils - This module serializes Markup into a plain string. There is a placeholder implementation that converts formatting like bolding into markup bolding (i.e **). This implementation isn't actually useful, because with our current use-cases, if we serialize, then there isn't any formatting to convert. I've left this placeholder implementation in case we need to have an opinionated serialization of rich text.
  4. translateVariants - This is a skeleton file for us to implement automatic translation of text in the future, as per Mike's designs.

Setup information

N/A

Deployment Notes

Requires changes in base-types.

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 force-pushed the brennan/text-variant/PL3-50 branch from e3c455a to cace4a4 Compare August 1, 2023 17:31
@zhihil zhihil self-assigned this Aug 8, 2023
if (!this.cachedCondition) {
this.cachedCondition = buildCondition(this.rawVariant.condition, this.varContext);
}
return this.cachedCondition;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This caching might be unnecessary in hindsight? This will essentially just save one object constructor call per variant.

default:
throw new VError('unexpected value for card layout');
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably could just turn this function into a map.

import { BaseLanguageGeneratorReturn } from './base.interface';
import { AIBillingEvents } from './billed.interface';

export class BilledGenerator<
Copy link
Contributor Author

@zhihil zhihil Aug 17, 2023

Choose a reason for hiding this comment

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

This class is essentially a "Dynamic Decorator" pattern.

The design problem here is that I need to inject billing utilities into origGenerator such that:

  1. The origGenerator is abstracted from the billing calls
  2. If the tokens are exceeded, then the .generate() call is never called. There isn't a perfectly clean abstraction using events and callbacks that permits this behaviour. I don't think .generate() should be aware that it has to not execute if the tokens run out. Therefore, it makes the most sense to put the billing in a wrapper method also called .generate()
  3. The billing calls are dependent on a dynamic value, the runtime, which may differ with each request.

A decorator like AuthGuard provides the right abstraction, but this is a compile-time construct that doesn't really fit the constraints above.

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

1 participant