From c10a40db995aa3847a5b29b051aed86d0d2aad00 Mon Sep 17 00:00:00 2001 From: Patrick Devine Date: Mon, 15 Sep 2025 18:09:05 -0700 Subject: [PATCH] parser: tidy up parameter/message parsing This change addresses how parameters and messages are handled while parsing a Modelfile. Currently a `MESSAGE` command creates a string that looks like ": " which then has to be re-parsed, and `PARAMETER` ends up being the default data type which makes it difficult to add other multi-part commands. This change introduces a Message and a Parameter type which properly handle properties such as the role and the name of the parameter. --- parser/parser.go | 110 +++++++++++++++++++++++++++++++----------- parser/parser_test.go | 34 ++++++------- 2 files changed, 98 insertions(+), 46 deletions(-) diff --git a/parser/parser.go b/parser/parser.go index e080f1bb7..30ebf9448 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -62,14 +62,15 @@ func (f Modelfile) CreateRequest(relativeDir string) (*api.CreateRequest, error) for _, c := range f.Commands { switch c.Name { case "model": - path, err := expandPath(c.Args, relativeDir) + name := c.Args.(string) + path, err := expandPath(name, relativeDir) if err != nil { return nil, err } digestMap, err := fileDigestMap(path) if errors.Is(err, os.ErrNotExist) { - req.From = c.Args + req.From = name continue } else if err != nil { return nil, err @@ -83,7 +84,8 @@ func (f Modelfile) CreateRequest(relativeDir string) (*api.CreateRequest, error) } } case "adapter": - path, err := expandPath(c.Args, relativeDir) + adapter := c.Args.(string) + path, err := expandPath(adapter, relativeDir) if err != nil { return nil, err } @@ -95,21 +97,25 @@ func (f Modelfile) CreateRequest(relativeDir string) (*api.CreateRequest, error) req.Adapters = digestMap case "template": - req.Template = c.Args + template := c.Args.(string) + req.Template = template case "system": - req.System = c.Args + system := c.Args.(string) + req.System = system case "license": - licenses = append(licenses, c.Args) + license := c.Args.(string) + licenses = append(licenses, license) case "message": - role, msg, _ := strings.Cut(c.Args, ": ") - messages = append(messages, api.Message{Role: role, Content: msg}) - default: + msg := c.Args.(*Message) + messages = append(messages, api.Message{Role: msg.Role, Content: msg.Content}) + case "parameter": if slices.Contains(deprecatedParameters, c.Name) { - fmt.Printf("warning: parameter %s is deprecated\n", c.Name) + fmt.Printf("warning: parameter '%s' is deprecated\n", c.Name) break } - ps, err := api.FormatParams(map[string][]string{c.Name: {c.Args}}) + param := c.Args.(*Parameter) + ps, err := api.FormatParams(map[string][]string{param.Name: {param.Value}}) if err != nil { return nil, err } @@ -123,6 +129,8 @@ func (f Modelfile) CreateRequest(relativeDir string) (*api.CreateRequest, error) params[k] = v } } + default: + return nil, fmt.Errorf("warning: unknown command '%s'", c.Name) } } @@ -312,7 +320,17 @@ func filesForModel(path string) ([]string, error) { type Command struct { Name string - Args string + Args any +} + +type Parameter struct { + Name string + Value string +} + +type Message struct { + Role string + Content string } func (c Command) String() string { @@ -321,12 +339,16 @@ func (c Command) String() string { case "model": fmt.Fprintf(&sb, "FROM %s", c.Args) case "license", "template", "system", "adapter": - fmt.Fprintf(&sb, "%s %s", strings.ToUpper(c.Name), quote(c.Args)) + data := c.Args.(string) + fmt.Fprintf(&sb, "%s %s", strings.ToUpper(c.Name), quote(data)) case "message": - role, message, _ := strings.Cut(c.Args, ": ") - fmt.Fprintf(&sb, "MESSAGE %s %s", role, quote(message)) + data := c.Args.(*Message) + fmt.Fprintf(&sb, "MESSAGE %s %s", data.Role, quote(data.Content)) + case "parameter": + data := c.Args.(*Parameter) + fmt.Fprintf(&sb, "PARAMETER %s %s", data.Name, quote(data.Value)) default: - fmt.Fprintf(&sb, "PARAMETER %s %s", c.Name, quote(c.Args)) + fmt.Printf("unknown command '%s'\n", c.Name) } return sb.String() @@ -366,7 +388,6 @@ func ParseFile(r io.Reader) (*Modelfile, error) { var curr state var currLine int = 1 var b bytes.Buffer - var role string var f Modelfile @@ -413,6 +434,7 @@ func ParseFile(r io.Reader) (*Modelfile, error) { case "parameter": // transition to stateParameter which sets command name next = stateParameter + cmd.Name = s case "message": // transition to stateMessage which validates the message role next = stateMessage @@ -421,16 +443,37 @@ func ParseFile(r io.Reader) (*Modelfile, error) { cmd.Name = s } case stateParameter: - cmd.Name = b.String() + s, ok := unquote(strings.TrimSpace(b.String())) + if !ok || isSpace(r) { + if _, err := b.WriteRune(r); err != nil { + return nil, err + } + + continue + } + cmd.Args = &Parameter{ + Name: s, + } case stateMessage: - if !isValidMessageRole(b.String()) { + s, ok := unquote(strings.TrimSpace(b.String())) + if !ok || isSpace(r) { + if _, err := b.WriteRune(r); err != nil { + return nil, err + } + + continue + } + + if !isValidMessageRole(s) { return nil, &ParserError{ LineNumber: currLine, Msg: errInvalidMessageRole.Error(), } } - role = b.String() + cmd.Args = &Message{ + Role: s, + } case stateComment, stateNil: // pass case stateValue: @@ -443,12 +486,16 @@ func ParseFile(r io.Reader) (*Modelfile, error) { continue } - if role != "" { - s = role + ": " + s - role = "" + switch cmd.Name { + case "parameter": + p := cmd.Args.(*Parameter) + p.Value = s + case "message": + m := cmd.Args.(*Message) + m.Content = s + default: + cmd.Args = s } - - cmd.Args = s f.Commands = append(f.Commands, cmd) } @@ -473,11 +520,16 @@ func ParseFile(r io.Reader) (*Modelfile, error) { return nil, io.ErrUnexpectedEOF } - if role != "" { - s = role + ": " + s + switch cmd.Name { + case "parameter": + c := cmd.Args.(*Parameter) + c.Value = s + case "message": + c := cmd.Args.(*Message) + c.Content = s + default: + cmd.Args = s } - - cmd.Args = s f.Commands = append(f.Commands, cmd) default: return nil, io.ErrUnexpectedEOF diff --git a/parser/parser_test.go b/parser/parser_test.go index 7d5a808ba..a2f1e1bd8 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -47,8 +47,8 @@ TEMPLATE """{{ if .System }}<|start_header_id|>system<|end_header_id|> {Name: "model", Args: "model1"}, {Name: "adapter", Args: "adapter1"}, {Name: "license", Args: "MIT"}, - {Name: "param1", Args: "value1"}, - {Name: "param2", Args: "value2"}, + {Name: "parameter", Args: &Parameter{"param1", "value1"}}, + {Name: "parameter", Args: &Parameter{"param2", "value2"}}, {Name: "template", Args: "{{ if .System }}<|start_header_id|>system<|end_header_id|>\n\n{{ .System }}<|eot_id|>{{ end }}{{ if .Prompt }}<|start_header_id|>user<|end_header_id|>\n\n{{ .Prompt }}<|eot_id|>{{ end }}<|start_header_id|>assistant<|end_header_id|>\n\n{{ .Response }}<|eot_id|>"}, } @@ -80,8 +80,8 @@ TEMPLATE """ {{ if .System }}<|start_header_id|>system<|end_header_id|> {Name: "model", Args: " model 1"}, {Name: "adapter", Args: "adapter3"}, {Name: "license", Args: "MIT "}, - {Name: "param1", Args: "value1"}, - {Name: "param2", Args: "value2"}, + {Name: "parameter", Args: &Parameter{"param1", "value1"}}, + {Name: "parameter", Args: &Parameter{"param2", "value2"}}, {Name: "template", Args: " {{ if .System }}<|start_header_id|>system<|end_header_id|>\n\n{{ .System }}<|eot_id|>{{ end }}{{ if .Prompt }}<|start_header_id|>user<|end_header_id|>\n\n{{ .Prompt }}<|eot_id|>{{ end }}<|start_header_id|>assistant<|end_header_id|>\n\n{{ .Response }}<|eot_id|> "}, } @@ -101,7 +101,7 @@ func TestParseFileFrom(t *testing.T) { }, { "FROM \"FOO BAR\"\nPARAMETER param1 value1", - []Command{{Name: "model", Args: "FOO BAR"}, {Name: "param1", Args: "value1"}}, + []Command{{Name: "model", Args: "FOO BAR"}, {Name: "parameter", Args: &Parameter{"param1", "value1"}}}, nil, }, { @@ -149,12 +149,12 @@ func TestParseFileFrom(t *testing.T) { }, { "PARAMETER param1 value1\nFROM foo", - []Command{{Name: "param1", Args: "value1"}, {Name: "model", Args: "foo"}}, + []Command{{Name: "parameter", Args: &Parameter{"param1", "value1"}}, {Name: "model", Args: "foo"}}, nil, }, { "PARAMETER what the \nFROM lemons make lemonade ", - []Command{{Name: "what", Args: "the"}, {Name: "model", Args: "lemons make lemonade"}}, + []Command{{Name: "parameter", Args: &Parameter{"what", "the"}}, {Name: "model", Args: "lemons make lemonade"}}, nil, }, } @@ -211,7 +211,7 @@ MESSAGE system You are a file parser. Always parse things. `, []Command{ {Name: "model", Args: "foo"}, - {Name: "message", Args: "system: You are a file parser. Always parse things."}, + {Name: "message", Args: &Message{"system", "You are a file parser. Always parse things."}}, }, nil, }, @@ -221,7 +221,7 @@ FROM foo MESSAGE system You are a file parser. Always parse things.`, []Command{ {Name: "model", Args: "foo"}, - {Name: "message", Args: "system: You are a file parser. Always parse things."}, + {Name: "message", Args: &Message{"system", "You are a file parser. Always parse things."}}, }, nil, }, @@ -234,9 +234,9 @@ MESSAGE assistant Hello, I want to parse all the things! `, []Command{ {Name: "model", Args: "foo"}, - {Name: "message", Args: "system: You are a file parser. Always parse things."}, - {Name: "message", Args: "user: Hey there!"}, - {Name: "message", Args: "assistant: Hello, I want to parse all the things!"}, + {Name: "message", Args: &Message{"system", "You are a file parser. Always parse things."}}, + {Name: "message", Args: &Message{"user", "Hey there!"}}, + {Name: "message", Args: &Message{"assistant", "Hello, I want to parse all the things!"}}, }, nil, }, @@ -244,12 +244,12 @@ MESSAGE assistant Hello, I want to parse all the things! ` FROM foo MESSAGE system """ -You are a multiline file parser. Always parse things. +You are a multiline file "parser". Always parse things. """ `, []Command{ {Name: "model", Args: "foo"}, - {Name: "message", Args: "system: \nYou are a multiline file parser. Always parse things.\n"}, + {Name: "message", Args: &Message{"system", "\nYou are a multiline file \"parser\". Always parse things.\n"}}, }, nil, }, @@ -514,7 +514,7 @@ func TestParseFileParameters(t *testing.T) { assert.Equal(t, []Command{ {Name: "model", Args: "foo"}, - {Name: v.name, Args: v.value}, + {Name: "parameter", Args: &Parameter{v.name, v.value}}, }, modelfile.Commands) }) } @@ -617,8 +617,8 @@ SYSTEM You are a utf16 file. expected := []Command{ {Name: "model", Args: "bob"}, - {Name: "param1", Args: "1"}, - {Name: "param2", Args: "4096"}, + {Name: "parameter", Args: &Parameter{"param1", "1"}}, + {Name: "parameter", Args: &Parameter{"param2", "4096"}}, {Name: "system", Args: "You are a utf16 file."}, }