Glebashnik/feed field generator #32842
Conversation
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class GenerateExpression extends Expression { |
There was a problem hiding this comment.
Please add @author and description.
| @Override | ||
| public DataType setInputType(DataType type, VerificationContext context) { | ||
| // TODO: Not sure if this implementation of the methods is correct, needs careful review. | ||
| super.setInputType(type, context); | ||
|
|
||
| if (type == DataType.STRING) | ||
| throw new IllegalArgumentException("generate requires a string input type, but got " + type); | ||
|
|
||
| return DataType.STRING; | ||
| } |
There was a problem hiding this comment.
| @Override | |
| public DataType setInputType(DataType type, VerificationContext context) { | |
| // TODO: Not sure if this implementation of the methods is correct, needs careful review. | |
| super.setInputType(type, context); | |
| if (type == DataType.STRING) | |
| throw new IllegalArgumentException("generate requires a string input type, but got " + type); | |
| return DataType.STRING; | |
| } | |
| @Override | |
| public DataType setInputType(DataType inputType, VerificationContext context) { | |
| return super.setInputType(inputType, DataType.STRING, context); | |
| } |
| @Override | ||
| public DataType setOutputType(DataType type, VerificationContext context) { | ||
| // TODO: Not sure if this implementation of the methods is correct, needs careful review. | ||
| super.setOutputType(type, type, context); | ||
|
|
||
| if (type != DataType.STRING) | ||
| throw new IllegalArgumentException("generate requires a string input type, but got " + type); | ||
|
|
||
| return DataType.STRING; | ||
| } |
There was a problem hiding this comment.
| @Override | |
| public DataType setOutputType(DataType type, VerificationContext context) { | |
| // TODO: Not sure if this implementation of the methods is correct, needs careful review. | |
| super.setOutputType(type, type, context); | |
| if (type != DataType.STRING) | |
| throw new IllegalArgumentException("generate requires a string input type, but got " + type); | |
| return DataType.STRING; | |
| } | |
| @Override | |
| public DataType setOutputType(DataType outputType, VerificationContext context) { | |
| return super.setOutputType(DataType.STRING, outputType, null, context); | |
| } |
| String generatorId, | ||
| List<String> generatorArguments | ||
| ) { | ||
| super(null); |
There was a problem hiding this comment.
| super(null); | |
| super(DataType.STRING); |
|
|
||
| if (!validTarget(targetType)) | ||
| throw new VerificationException(this, "The generate target field must be a String"); | ||
|
|
There was a problem hiding this comment.
| if (!validTarget(targetType)) | |
| throw new VerificationException(this, "The generate target field must be a String"); | |
| @@ -0,0 +1,29 @@ | |||
| package ai.vespa.generative; | |||
There was a problem hiding this comment.
Same, I think we could put this in llm.generation
And maybe, since what separates it from Generator isn't that it's generating from a language model, but that it's taking an additional config specifying a prompt, we could call it something like PromptedGenerator, or ConfiguredGenerator?
| return Map.of(name, this); | ||
| } | ||
|
|
||
| String generate(String prompt, Context context, DataType dataType); |
There was a problem hiding this comment.
Please remove the DataType argument, I don't think you need it and sine it is a document side concept it makes it weird to use this interface in other contexts.
| return Map.of(name, this); | ||
| } | ||
|
|
||
| String generate(String prompt, Context context, DataType dataType); |
There was a problem hiding this comment.
Why not use the Prompt class here so we have the option of passing prompts through that aren't just strings.
| @@ -31,7 +34,7 @@ | |||
| * | |||
| * @author lesters | |||
| */ | |||
| public class LocalLLM extends AbstractComponent implements LanguageModel { | |||
| public class LocalLLM extends AbstractComponent implements LanguageModel, Generator { | |||
There was a problem hiding this comment.
The role of Generator is to apply a LanguageModel to a generation task specified by a context. This is a more general API so it's a bit messy for it to know about that context (and also to call itself through a utility defined elsewhere). I see the purpose though ... maybe there's a better solution, but I need to get some breakfast now.
| @@ -33,6 +33,7 @@ import com.yahoo.text.StringUtilities; | |||
| import com.yahoo.vespa.indexinglanguage.expressions.*; | |||
There was a problem hiding this comment.
Unfortunately, atm we have two parsers. You need to also make these same additions in IndexingParser.ccc in schema-language-server.
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.