Skip to content

Commit

Permalink
Changes based on review feedback:
Browse files Browse the repository at this point in the history
 - Returning template for chaining method calls
 - Returning a `TextMessage` object instead of a tuple
  • Loading branch information
martindevans committed May 6, 2024
1 parent d7a8917 commit 939c5e0
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 17 deletions.
17 changes: 12 additions & 5 deletions LLama.Unittest/TemplateTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,21 @@ public void RemoveMid()
templater.Add("user", "4b");
templater.Add("assistant", "5");

Assert.Equal(("user", "4a"), templater[3]);
Assert.Equal(("assistant", "5"), templater[5]);
Assert.Equal("user", templater[3].Role);
Assert.Equal("4a", templater[3].Content);

Assert.Equal("assistant", templater[5].Role);
Assert.Equal("5", templater[5].Content);

Assert.Equal(6, templater.Count);
templater.RemoveAt(3);
Assert.Equal(5, templater.Count);

Assert.Equal(("user", "4b"), templater[3]);
Assert.Equal(("assistant", "5"), templater[4]);
Assert.Equal("user", templater[3].Role);
Assert.Equal("4b", templater[3].Content);

Assert.Equal("assistant", templater[4].Role);
Assert.Equal("5", templater[4].Content);
}

[Fact]
Expand All @@ -226,7 +232,8 @@ public void RemoveLast()
templater.RemoveAt(5);
Assert.Equal(5, templater.Count);

Assert.Equal(("user", "4b"), templater[4]);
Assert.Equal("user", templater[4].Role);
Assert.Equal("4b", templater[4].Content);
}

[Fact]
Expand Down
61 changes: 49 additions & 12 deletions LLama/LLamaTemplate.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public sealed class LLamaTemplate
/// <summary>
/// Array of messages. The <see cref="Count"/> property indicates how many messages there are
/// </summary>
private Message[] _messages = new Message[4];
private TextMessage?[] _messages = new TextMessage[4];

/// <summary>
/// Backing field for <see cref="AddAssistant"/>
Expand Down Expand Up @@ -71,7 +71,7 @@ public sealed class LLamaTemplate
/// <param name="index"></param>
/// <returns></returns>
/// <exception cref="ArgumentOutOfRangeException">Thrown if index is less than zero or greater than or equal to <see cref="Count"/></exception>
public (string role, string content) this[int index]
public TextMessage this[int index]
{
get
{
Expand All @@ -80,7 +80,7 @@ public sealed class LLamaTemplate
if (index >= Count)
throw new ArgumentOutOfRangeException(nameof(index), "Index must be < Count");

return (_messages[index].Role, _messages[index].Content);
return _messages[index]!;
}
}

Expand Down Expand Up @@ -131,30 +131,45 @@ public LLamaTemplate(string customTemplate)
}
#endregion

#region modify
/// <summary>
/// Add a new message to the end of this template
/// </summary>
/// <param name="role"></param>
/// <param name="content"></param>
public void Add(string role, string content)
/// <returns>This template, for chaining calls.</returns>
public LLamaTemplate Add(string role, string content)
{
return Add(new TextMessage(role, content, _roleCache));
}

/// <summary>
/// Add a new message to the end of this template
/// </summary>
/// <param name="message"></param>
/// <returns>This template, for chaining calls.</returns>
public LLamaTemplate Add(TextMessage message)
{
// Expand messages array if necessary
if (Count == _messages.Length)
Array.Resize(ref _messages, _messages.Length * 2);

// Add message
_messages[Count] = new Message(role, content, _roleCache);
_messages[Count] = message;
Count++;

// Mark as dirty to ensure template is recalculated
_dirty = true;

return this;
}

/// <summary>
/// Remove a message at the given index
/// </summary>
/// <param name="index"></param>
public void RemoveAt(int index)
/// <returns>This template, for chaining calls.</returns>
public LLamaTemplate RemoveAt(int index)
{
if (index < 0)
throw new ArgumentOutOfRangeException(nameof(index), "Index must be greater than or equal to zero");
Expand All @@ -169,7 +184,10 @@ public void RemoveAt(int index)
Array.Copy(_messages, index + 1, _messages, index, Count - index);

_messages[Count] = default;

return this;
}
#endregion

/// <summary>
/// Apply the template to the messages and write it into the output buffer
Expand All @@ -192,7 +210,8 @@ public int Apply(Memory<byte> dest)
Array.Resize(ref _nativeChatMessages, _messages.Length);
for (var i = 0; i < Count; i++)
{
ref var m = ref _messages[i];
ref var m = ref _messages[i]!;
Debug.Assert(m != null);
totalInputBytes += m.RoleBytes.Length + m.ContentBytes.Length;

Check warning on line 215 in LLama/LLamaTemplate.cs

View workflow job for this annotation

GitHub Actions / Test (linux-release)

Dereference of a possibly null reference.

Check warning on line 215 in LLama/LLamaTemplate.cs

View workflow job for this annotation

GitHub Actions / Test (osx-release)

Dereference of a possibly null reference.

// Pin byte arrays in place
Expand Down Expand Up @@ -258,17 +277,24 @@ unsafe int ApplyInternal(Span<LLamaChatMessage> messages, byte[] output)
}

/// <summary>
/// A message that has been added to the template, contains role and content converted into UTF8 bytes.
/// A message that has been added to a template
/// </summary>
private readonly record struct Message
public sealed class TextMessage
{
/// <summary>
/// The "role" string for this message
/// </summary>
public string Role { get; }

/// <summary>
/// The text content of this message
/// </summary>
public string Content { get; }

public ReadOnlyMemory<byte> RoleBytes { get; }
public ReadOnlyMemory<byte> ContentBytes { get; }
internal ReadOnlyMemory<byte> RoleBytes { get; }
internal ReadOnlyMemory<byte> ContentBytes { get; }

public Message(string role, string content, Dictionary<string, ReadOnlyMemory<byte>> roleCache)
internal TextMessage(string role, string content, IDictionary<string, ReadOnlyMemory<byte>> roleCache)
{
Role = role;
Content = content;
Expand Down Expand Up @@ -297,5 +323,16 @@ public Message(string role, string content, Dictionary<string, ReadOnlyMemory<by
Debug.Assert(contentArray.Length == encodedContentLength + 1);
ContentBytes = contentArray;
}

/// <summary>
/// Deconstruct this message into role and content
/// </summary>
/// <param name="role"></param>
/// <param name="content"></param>
public void Deconstruct(out string role, out string content)
{
role = Role;
content = Content;
}
}
}

0 comments on commit 939c5e0

Please sign in to comment.