Skip to content

fix: Models would sometimes output cell coordinates #167

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.

Sign up for GitHub

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

Merged
kaspermarstal merged 1 commit into from
May 13, 2025
Merged

fix: Models would sometimes output cell coordinates #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 88 additions & 28 deletions src/Cellm/AddIn/ArgumentParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Cellm.AddIn;

public class ArgumentParser
public class ArgumentParser(IConfiguration configuration)
{
private string? _provider;
private string? _model;
Expand All @@ -19,13 +19,6 @@ public class ArgumentParser
public static readonly string InstructionsStartTag = "<instructions>";
public static readonly string InstructionsEndTag = "<instructions>";

private readonly IConfiguration _configuration;

public ArgumentParser(IConfiguration configuration)
{
_configuration = configuration;
}

public ArgumentParser AddProvider(object providerAndModel)
{
_provider = providerAndModel switch
Expand Down Expand Up @@ -73,22 +66,22 @@ public ArgumentParser AddTemperature(object temperature)

public Arguments Parse()
{
var providerAsString = _provider ?? _configuration
var providerAsString = _provider ?? configuration
.GetSection(nameof(ProviderConfiguration))
.GetValue<string>(nameof(ProviderConfiguration.DefaultProvider))
?? throw new ArgumentException(nameof(ProviderConfiguration.DefaultProvider));

if (!Enum.TryParse<Provider>(providerAsString, true, out var provider))
{
throw new ArgumentException($"Unsupported default provider: {providerAsString}");
throw new ArgumentException($"Unsupported provider: {providerAsString}");
}

var model = _model ?? _configuration
var model = _model ?? configuration
.GetSection($"{provider}Configuration")
.GetValue<string>(nameof(IProviderConfiguration.DefaultModel))
?? throw new ArgumentException(nameof(IProviderConfiguration.DefaultModel));

var defaultTemperature = _configuration
var defaultTemperature = configuration
.GetSection(nameof(ProviderConfiguration))
.GetValue<double?>(nameof(ProviderConfiguration.DefaultTemperature))
?? throw new ArgumentException(nameof(ProviderConfiguration.DefaultTemperature));
Expand Down Expand Up @@ -150,6 +143,7 @@ private static string GetCellAsString(ExcelReference providerAndModel)
return providerAndModel.GetValue()?.ToString() ?? throw new ArgumentException("Provider and model argument must be a valid cell reference");
}

// Render sheet as Markdown table because models have seen loads of those
private static string ParseCells(ExcelReference reference)
{
try
Expand All @@ -159,32 +153,96 @@ private static string ParseCells(ExcelReference reference)
sheetName = sheetName[(sheetName.LastIndexOf(']') + 1)..];
var worksheet = app.Sheets[sheetName];

var tableBuilder = new StringBuilder();
var valueBuilder = new StringBuilder();
var numberOfRows = reference.RowLast - reference.RowFirst + 1; // 1-indexed
var numberOfColumns = reference.ColumnLast - reference.ColumnFirst + 1; // 1-indexed

var numberOfRenderedRows = numberOfRows + 1; // Includes the header row
var numberOfRenderedColumns = numberOfColumns + 1; // Includes the row-number column

var isEmpty = true;

// Column-major for easy calculation of padding
var table = new List<List<string>>();

// Initialize table with empty rows for reduced number of heap allocations
for (var c = 0; c < numberOfRenderedColumns; c++)
{
table.Add(new List<string>(numberOfRenderedRows));
}

// Add row number column
table[0].Add("Row \\ Col"); // Top-left

var rows = reference.RowLast - reference.RowFirst + 1;
var columns = reference.ColumnLast - reference.ColumnFirst + 1;
for (var r = 0; r < numberOfRows; r++)
{
table[0].Add(GetRowName(reference.RowFirst + r));
}

for (var row = 0; row < rows; row++)
// Add other columns
for (var c = 0; c < numberOfColumns; c++)
{
for (var column = 0; column < columns; column++)
table[c + 1].Add(GetColumnName(reference.ColumnFirst + c)); // Column header

for (var r = 0; r < numberOfRows; r++)
{
var value = worksheet.Cells[reference.RowFirst + row + 1, reference.ColumnFirst + column + 1].Text;
valueBuilder.Append(value);
var value = worksheet.Cells[reference.RowFirst + r + 1, reference.ColumnFirst + c + 1].Text?.ToString() ?? string.Empty;

tableBuilder.Append("| ");
tableBuilder.Append(GetColumnName(reference.ColumnFirst + column) + GetRowName(reference.RowFirst + row));
tableBuilder.Append(' ');
tableBuilder.Append(value);
if (isEmpty && !string.IsNullOrEmpty(value))
{
isEmpty = false;
}

string sanitizedCellValue = value.Replace("\r\n", " ").Replace("\n", " ").Replace("|", "\\|");
table[c + 1].Add(sanitizedCellValue);
}
}

// Pad columns
foreach (var column in table)
{
var maxWidth = column.Max(cell => cell?.Length ?? 0);

for (var r = 0; r < column.Count; r++)
{
column[r] = column[r].PadRight(maxWidth);
}
}

var tableBuilder = new StringBuilder();

// Iterate row-major for StringBuilder
for (var r = 0; r < numberOfRenderedRows; r++)
{
tableBuilder.Append('|');

for (var c = 0; c < numberOfRenderedColumns; c++)
{
tableBuilder.Append(' ');
tableBuilder.Append(table[c][r]); // Access [column][row]
tableBuilder.Append(" |");
}

tableBuilder.AppendLine("|");
tableBuilder.AppendLine();

// Add separator line after the header row (r == 0)
if (r == 0)
{
tableBuilder.Append('|');
for (var c = 0; c < numberOfRenderedColumns; c++)
{
tableBuilder.Append(' ');
// Length of separator is based on the padded header cell's length
tableBuilder.Append(new string('-', table[c][r].Length));
tableBuilder.Append(" |");
}

tableBuilder.AppendLine();
}
}

if (string.IsNullOrEmpty(valueBuilder.ToString()))
if (isEmpty)
{
throw new ArgumentException("Empty cells");
throw new ArgumentNullException(nameof(reference));
}

return tableBuilder.ToString();
Expand All @@ -197,12 +255,14 @@ private static string ParseCells(ExcelReference reference)

private static string GetColumnName(int columnNumber)
{
string columnName = "";
var columnName = string.Empty;

while (columnNumber >= 0)
{
columnName = (char)('A' + columnNumber % 26) + columnName;
columnNumber = columnNumber / 26 - 1;
}

return columnName;
}

Expand Down
19 changes: 12 additions & 7 deletions src/Cellm/AddIn/CellmFunctions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
using System.Diagnostics;
using System.Text;
using System.Text;
using Cellm.AddIn.Exceptions;
using Cellm.Models.Prompts;
using Cellm.Models.Providers;
using ExcelDna.Integration;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace Cellm.AddIn;
Expand Down Expand Up @@ -76,7 +76,6 @@ public static object PromptWith(
IsCellReferenceGettingData(instructionsOrContext) ||
IsCellReferenceGettingData(instructionsOrTemperature))
{
Debug.WriteLine("PromptWith: Detected #GETTING_DATA in one of the resolved input values. Returning ExcelError.ExcelErrorGettingData.");
return ExcelError.ExcelErrorGettingData;
}

Expand Down Expand Up @@ -111,11 +110,17 @@ public static object PromptWith(
new object[] { providerAndModel, instructionsOrContext, instructionsOrTemperature, temperature },
() => new ObserveResponse(prompt, arguments.Provider));
}
catch (CellmException e)
catch (CellmException ex)
{
SentrySdk.CaptureException(e);
Debug.WriteLine(e);
return e.Message;
SentrySdk.CaptureException(ex);

var logger = CellmAddIn.Services
.GetRequiredService<ILoggerFactory>()
.CreateLogger(nameof(PromptWith));

logger.LogError(ex, "{method} failed", nameof(PromptWith));

return ExcelError.ExcelErrorValue;
}
}

Expand Down