@@ 159,10 159,20 @@ type readRowsOutput struct {
// Rows are rendered strings and not typed values: this surface reads a bare
// store without a SQL engine, so what it has is the stored value formatted.
- // A NULL cell reads as "NULL" and an unprintable one as "<binary>", neither
- // of which is distinguishable from a row that stores those very strings —
- // which is a limit worth knowing rather than a bug to work around.
- Rows [][]string `json:"rows"`
+ //
+ // A cell that holds no value is null rather than a string. browse renders a
+ // real NULL as the text "NULL", which is exactly what a row storing those
+ // four characters renders as — harmless on a page, where a reader sees the
+ // same thing either way, and not harmless here: this tool hands rows to a
+ // machine with no schema beside them, and "is there a value in this cell?"
+ // is not a question an agent can answer from the string afterwards. The mask
+ // browse carries beside its rows is what makes the two distinguishable, and
+ // spending it on a JSON null is the shape that needs no explaining.
+ //
+ // An unprintable value still reads as "<binary>" and is still not
+ // distinguishable from a row that stores that text. That one stays: it
+ // answers what a value *is*, and the caller can see it is there.
+ Rows [][]*string `json:"rows"`
Offset int `json:"offset"`
@@ 298,8 308,8 @@ func (s *Server) registerBrowseTools() {
Description: "Read one page of rows from one table of a hosted Dolt database.\n\n" +
"`columns` names the cells of every row in order (primary-key columns first for a keyed " +
"table). Cells are rendered strings, because this surface reads a bare store without a SQL " +
- "engine: a NULL cell reads as `NULL` and an unprintable one as `<binary>`, neither " +
- "distinguishable from a row that stores those strings literally.\n\n" +
+ "engine: a cell that holds no value is `null`, and an unprintable one is the string " +
+ "`<binary>`.\n\n" +
"`limit` defaults to 100 and is capped at 500; the `limit` in the answer is the one actually " +
"applied, so a larger request is visibly answered at the cap. `total` is the number of rows " +
"in the whole table, and `truncated` says rows remain after this page — read them by raising " +
@@ 471,7 481,7 @@ func (s *Server) readRows(ctx context.Context, in readRowsInput) (readRowsOutput
Ref: ref,
Table: table,
Columns: page.Columns,
- Rows: page.Rows,
+ Rows: nullableCells(page),
Offset: page.Offset,
Limit: limit,
Total: page.Total,
@@ 484,12 494,40 @@ func (s *Server) readRows(ctx context.Context, in readRowsInput) (readRowsOutput
if out.Columns == nil {
out.Columns = []string{}
}
- if out.Rows == nil {
- out.Rows = [][]string{}
- }
return out, nil
}
+// nullableCells reads a page's rows through the NULL mask beside them: a cell
+// the mask reports as holding no value becomes a nil pointer, which is the JSON
+// null a caller decodes, and every other cell is the string browse rendered.
+//
+// It never returns nil, so an empty page answers an empty array rather than a
+// null one — the rule read.go states for every list on this surface.
+//
+// A page whose mask is missing, or shorter than its rows, reports values and
+// never nulls. browse builds a mask parallel to the rows for every page it
+// returns, so that arm is about a page this package did not get from browse;
+// claiming an absence for a cell nothing was observed about would be inventing
+// the very answer the mask exists to carry.
+func nullableCells(page *browse.RowPage) [][]*string {
+ out := make([][]*string, 0, len(page.Rows))
+ for i, row := range page.Rows {
+ var mask []bool
+ if i < len(page.Nulls) {
+ mask = page.Nulls[i]
+ }
+ cells := make([]*string, len(row))
+ for j, v := range row {
+ if j < len(mask) && mask[j] {
+ continue // nil: this cell holds no value at all
+ }
+ cells[j] = &v
+ }
+ out = append(out, cells)
+ }
+ return out
+}
+
// getCommitLog answers get_commit_log: a page of history with the cursor that
// continues it.
func (s *Server) getCommitLog(ctx context.Context, in getCommitLogInput) (getCommitLogOutput, error) {
@@ 0,0 1,112 @@
+package mcpsrv_test
+
+import (
+ "encoding/json"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "sourcecraft.dev/bigbes/sr-ht-dolt/browse"
+ "sourcecraft.dev/bigbes/sr-ht-dolt/core"
+ "sourcecraft.dev/bigbes/sr-ht-dolt/mcpsrv"
+)
+
+// read_rows and the one question a rendered string cannot answer: is there a
+// value in this cell at all?
+//
+// browse renders a real NULL as the text "NULL", which is what a row storing
+// those four characters renders as too. On a page the two are interchangeable —
+// a reader sees the same thing either way — but this tool hands rows to a
+// machine with no schema beside them, and an agent cannot recover the
+// distinction afterwards. So the mask browse carries beside its rows is spent
+// here: an absent value is JSON null, a stored "NULL" is the string.
+
+// nullsTable is a table holding both halves of the ambiguity in one column:
+// row 2 stores no value, row 3 stores the text "NULL", and browse renders both
+// as "NULL". Only the mask tells them apart.
+func nullsTable() fakeTable {
+ return fakeTable{
+ name: "cells",
+ cols: []browse.ColumnInfo{
+ {Name: "id", Type: "int", PrimaryKey: true},
+ {Name: "body", Type: "text", Nullable: true},
+ },
+ rows: [][]string{
+ {"1", "plain"},
+ {"2", "NULL"}, // no value: browse rendered the placeholder
+ {"3", "NULL"}, // the four characters, actually stored
+ },
+ nulls: [][]bool{
+ {false, false},
+ {false, true},
+ {false, false},
+ },
+ }
+}
+
+// nullableRowsResult is read_rows decoded into cells that can be null, which is
+// the shape this tool answers: [][]*string, where a nil cell is a JSON null.
+type nullableRowsResult struct {
+ Columns []string `json:"columns"`
+ Rows [][]*string `json:"rows"`
+}
+
+func TestReadRowsAnswersARealNullAsNullAndAStoredNullAsAString(t *testing.T) {
+ res := call(t, connect(t, nullsSurface(t), nil), "read_rows", args("cells", "table", "cells"))
+ require.False(t, res.IsError, "%s", errorText(res))
+
+ var out nullableRowsResult
+ decode(t, res, &out)
+
+ require.Equal(t, []string{"id", "body"}, out.Columns)
+ require.Len(t, out.Rows, 3)
+
+ require.NotNil(t, out.Rows[0][1])
+ assert.Equal(t, "plain", *out.Rows[0][1])
+
+ assert.Nil(t, out.Rows[1][1], "a cell that holds no value is null, not the string \"NULL\"")
+
+ require.NotNil(t, out.Rows[2][1], "a row that stores the text \"NULL\" still has a value")
+ assert.Equal(t, "NULL", *out.Rows[2][1])
+
+ // A key is never null, and neither is a cell that holds an ordinary value:
+ // the mask is spent on the one column that needed it and nowhere else.
+ for i, row := range out.Rows {
+ require.NotNilf(t, row[0], "row %d: the primary key holds a value", i)
+ }
+}
+
+// The distinction has to survive the wire, not merely the Go struct: what an
+// agent reads is the JSON, so the null is asserted there too.
+func TestReadRowsPutsANullOnTheWire(t *testing.T) {
+ res := call(t, connect(t, nullsSurface(t), nil), "read_rows", args("cells", "table", "cells"))
+ require.False(t, res.IsError, "%s", errorText(res))
+ require.NotNil(t, res.StructuredContent)
+
+ raw, err := json.Marshal(res.StructuredContent)
+ require.NoError(t, err)
+ assert.Contains(t, string(raw), `[["1","plain"],["2",null],["3","NULL"]]`,
+ "the absent value is a JSON null and the stored text is a string")
+}
+
+// nullsSurface is the server over the cells database.
+func nullsSurface(t *testing.T) *mcpsrv.Server {
+ t.Helper()
+
+ repos := &fakeRepos{acl: map[int]map[int]core.AccessMode{}}
+ repos.repos = append(repos.repos, &core.Repo{
+ ID: 1,
+ Name: "cells",
+ OwnerID: aliceID,
+ OwnerName: "alice",
+ Path: storePath("alice", "cells"),
+ Visibility: core.VisibilityPublic,
+ })
+
+ opener := &fakeOpener{sessions: map[string]*fakeSession{
+ storePath("alice", "cells"): smallStore("main", "cccc", []fakeTable{nullsTable()}),
+ }}
+
+ return newServer(t, repos, opener)
+}
@@ 200,6 200,12 @@ type fakeTable struct {
name string
cols []browse.ColumnInfo
rows [][]string
+
+ // nulls is the NULL mask browse carries beside its rows, parallel to rows.
+ // A table that leaves it unset holds a value in every cell, and pageOf builds
+ // the all-false mask browse would have returned for it — a fixture never
+ // answers a page without a mask, because the store never does either.
+ nulls [][]bool
}
func (t fakeTable) info() browse.TableInfo {
@@ 596,16 602,35 @@ func pageOf(t fakeTable, offset, limit int) *browse.RowPage {
page := &browse.RowPage{
Columns: t.columns(),
Rows: [][]string{},
+ Nulls: [][]bool{},
Offset: offset,
Total: len(t.rows),
}
if offset < len(t.rows) {
end := min(offset+limit, len(t.rows))
page.Rows = append(page.Rows, t.rows[offset:end]...)
+ page.Nulls = append(page.Nulls, t.mask(offset, end)...)
}
return page
}
+// mask is the NULL mask of rows [lo, hi), sliced in lockstep with them. A table
+// that declared none holds no NULL anywhere, so the mask is all false — the
+// shape browse returns for such a table, rather than an absent one.
+func (t fakeTable) mask(lo, hi int) [][]bool {
+ if t.nulls != nil {
+ if len(t.nulls) != len(t.rows) {
+ panic("fake: the NULL mask of " + t.name + " is not parallel to its rows")
+ }
+ return t.nulls[lo:hi]
+ }
+ out := make([][]bool, 0, hi-lo)
+ for _, r := range t.rows[lo:hi] {
+ out = append(out, make([]bool, len(r)))
+ }
+ return out
+}
+
func (s *fakeSession) CommitSummary(_ context.Context, hashStr string) (*browse.CommitDiff, error) {
hash := s.resolve(hashStr)
if hash == "" {