Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Read in this order, and stop as soon as you have what you need:
Go way instead, as `// Command sqlb …` at the head of `cmd/sqlb/main.go`.
2. **[docs/architecture.md](docs/architecture.md)** for how the pieces fit and
why the seams are where they are.
3. **[docs/adr/](docs/adr/)** — 50 records, and they are *load-bearing rather
3. **[docs/adr/](docs/adr/)** — 51 records, and they are *load-bearing rather
than historical*. A decision here is usually the answer to "why is this not
simpler", and reversing one without reading it is the most common way to
spend an afternoon rediscovering a rejected alternative. Each carries a
Expand Down
29 changes: 22 additions & 7 deletions codegen/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,15 @@ func renderActions(b *bytes.Buffer, defs []actionDef) {
fmt.Fprintf(b, "\t//\n\t// %s\n", desc)
}
if w := d.action.Writes; len(w) > 0 {
fmt.Fprintf(b, "\t//\n\t// The envelope persists %s afterwards, and nothing else.\n",
quoteList(w))
fmt.Fprintf(b, "\t//\n\t// The envelope persists %s off this row afterwards, and nothing\n"+
"\t// else — which bounds the envelope and not the func: the transaction\n"+
"\t// is yours through sqlb.TxFrom, and statements issued there take\n"+
"\t// their own locks, in an order this code owns.\n", quoteList(w))
}
if tt := d.action.Touches; len(tt) > 0 {
fmt.Fprintf(b, "\t//\n\t// Declared reach beyond that row: %s. Nothing checks it; it is\n"+
"\t// what the route tells `sqlb impact`, the OpenAPI document and the\n"+
"\t// CLI's --help, so a change here belongs in the schema.\n", quoteList(tt))
}
if d.action.IsCollection() {
fmt.Fprintf(b, "\t%s func(context.Context, %s) error\n", d.goName(), d.inputName())
Expand Down Expand Up @@ -201,11 +208,10 @@ func renderActionCalls(b *bytes.Buffer, optsVar string, defs []actionDef) {
fmt.Fprintf(b, "\t\tDescription: %q,\n", s)
}
if w := d.action.Writes; len(w) > 0 {
quoted := make([]string, len(w))
for i, name := range w {
quoted[i] = fmt.Sprintf("%q", name)
}
fmt.Fprintf(b, "\t\tWrites: []string{%s},\n", strings.Join(quoted, ", "))
fmt.Fprintf(b, "\t\tWrites: []string{%s},\n", quotedList(w))
}
if tt := d.action.Touches; len(tt) > 0 {
fmt.Fprintf(b, "\t\tTouches: []string{%s},\n", quotedList(tt))
}
if len(d.action.Body) > 0 {
fmt.Fprintf(b, "\t\tHasBody: true,\n")
Expand All @@ -214,6 +220,15 @@ func renderActionCalls(b *bytes.Buffer, optsVar string, defs []actionDef) {
}
}

// quotedList renders a name set as Go source: "status", "closed_at".
func quotedList(names []string) string {
quoted := make([]string, len(names))
for i, name := range names {
quoted[i] = fmt.Sprintf("%q", name)
}
return strings.Join(quoted, ", ")
}

// quoteList renders a column set for a doc comment: `status` and `closed_at`.
func quoteList(names []string) string {
quoted := make([]string, len(names))
Expand Down
62 changes: 58 additions & 4 deletions codegen/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,18 @@ func actionFixture() *schema.Registry {
schema.Timestamp("completed_at"),
),
Writes: []string{"status", "closed_at"},
// The verb also writes a comment row through the transaction,
// which the write set above cannot say and used to leave unsaid.
Touches: []string{"comments"},
}).
Action(schema.Action{
Name: "archive",
Writes: []string{"status"},
}).
Action(schema.Action{
Name: "purge-archived",
Path: "/purge-archived",
Name: "purge-archived",
Path: "/purge-archived",
Touches: []string{"tasks", "comments"},
})
return r
}
Expand Down Expand Up @@ -106,6 +110,37 @@ func TestActionsStructNamesEveryVerbWithItsSignature(t *testing.T) {
}
}

// The declared reach reaches the runtime spec and the doc comment above the
// func the application writes.
//
// Both matter and for different readers: the spec is what puts the sentence in
// the OpenAPI document, and the comment is what the author of the verb sees at
// the moment they are deciding whether to reach for sqlb.TxFrom (#149).
func TestTheDeclaredReachReachesTheGeneratedCode(t *testing.T) {
src := generate(t, actionFixture())["rest_gen.go"]

if !strings.Contains(specOf(t, src, "complete"), `Touches: []string{"comments"}`) {
t.Errorf("the action's spec does not carry its reach:\n%s", specOf(t, src, "complete"))
}
// A verb with no declared reach emits no field, so a schema that never uses
// this generates what it generated before.
if strings.Contains(specOf(t, src, "archive"), "Touches") {
t.Error("a verb with no declared reach emitted a Touches field")
}

for _, want := range []string{
"// Declared reach beyond that row: `comments`.",
// And the write set now says what it bounds, which is the envelope and
// not the transaction the func is handed.
"the transaction",
"is yours through sqlb.TxFrom",
} {
if !strings.Contains(src, want) {
t.Errorf("the Actions doc comment is missing %q:\n%s", want, src)
}
}
}

// A schema with no verbs must generate exactly what it generated before this
// feature existed. The parameter is additive; the absence of it is the promise.
func TestRegisterKeepsItsSignatureWhenNothingDeclaresAnAction(t *testing.T) {
Expand Down Expand Up @@ -230,13 +265,32 @@ func TestActionsReachTheCLI(t *testing.T) {
// round trip less than relaying the server's 422.
`_ = cmd.MarkFlagRequired("completed-at")`,
// The write set is in the help, because "what does this touch" is the
// question an operator has at the prompt.
"This writes status, closed_at, and no other column.",
// question an operator has at the prompt. It is stated as what comes
// back on the row rather than as the blast radius, which it is not.
"The response row carries status, closed_at, and no other column the server changed on it.",
// And the declared reach beside it, which is the half the write set
// understates. --help is the surface ADR-0029 argues about hardest: a
// caller with no compile step reads this instead of making a request.
"Beyond that row the route writes: comments.",
"The schema declares that set; nothing enforces it.",
// A collection verb has no row at all, so the reach is the only thing
// it can state.
"Beyond that row the route writes: tasks, comments.",
} {
if !strings.Contains(src, want) {
t.Errorf("the CLI does not contain %q\n%s", want, src)
}
}
// An undeclared reach is not a bound. `archive` names no Touches, and its
// help says so rather than leaving the write set to be read as the whole
// of it — which is the misreading the issue reported.
archiveHelp := src[strings.Index(src, "newTasksArchiveCommand(c *client.Client)"):]
if end := strings.Index(archiveHelp, "return cmd"); end > 0 {
archiveHelp = archiveHelp[:end]
}
if !strings.Contains(archiveHelp, "the absence of a claim rather") {
t.Errorf("a verb with no declared reach should say so:\n%s", archiveHelp)
}
// A verb with no body sends none, rather than posting `{}` at an operation
// that does not declare one.
archive := src[strings.Index(src, "newTasksArchiveCommand(c *client.Client)"):]
Expand Down
22 changes: 21 additions & 1 deletion codegen/cliaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,35 @@ func cliActionLong(r cliResource, a schema.Action) string {
}
if a.IsCollection() {
b.WriteString("A verb on the collection: it addresses no single row, and a successful call\nwrites nothing to print.")
writeCLIReach(&b, a)
return b.String()
}
b.WriteString("A verb on one row. The server fetches it, runs the transition, and answers\nwith the row as it now stands.")
if len(a.Writes) > 0 {
fmt.Fprintf(&b, "\n\nThis writes %s, and no other column.", strings.Join(a.Writes, ", "))
fmt.Fprintf(&b, "\n\nThe response row carries %s, and no other column the server changed on it.",
strings.Join(a.Writes, ", "))
}
writeCLIReach(&b, a)
return b.String()
}

// writeCLIReach states what the verb writes beyond the row it answers with.
//
// This is the surface ADR-0029's argument is sharpest about: --help is what a
// caller with no compile step reads instead of a request, and a caller reading
// a write set of two columns concludes the verb is confined to one row. The
// sentence goes in whether or not the schema declared a reach, because the
// absence of a declaration is not the absence of a reach — an undeclared verb
// still holds the transaction.
func writeCLIReach(b *strings.Builder, a schema.Action) {
if len(a.Touches) > 0 {
fmt.Fprintf(b, "\n\nBeyond that row the route writes: %s.\nThe schema declares that set; nothing enforces it.",
strings.Join(a.Touches, ", "))
return
}
b.WriteString("\n\nThe route declares nothing written beyond that row. A verb holds the\ntransaction and may write more, so this is the absence of a claim rather\nthan a checked bound.")
}

// cliActionExample writes one runnable invocation, filling every required flag.
func cliActionExample(r cliResource, a schema.Action) string {
var args []string
Expand Down
12 changes: 10 additions & 2 deletions codegen/computed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,32 @@ func computedFixture() *schema.Registry {
schema.Date("due_date").Nullable().Filterable().Sortable(),
schema.Int("open_tasks").Filterable(),

// Nullable, and not by an oversight: due_date is nullable, so the
// comparison is NULL for every row without one.
schema.Computed("is_overdue", schema.TypeBool,
schema.FromSQL("due_date < current_date AND open_tasks > 0")).Filterable(),
// EXISTS is true or false and never NULL, which is what NotNull claims.
schema.Computed("is_starred", schema.TypeBool,
schema.FromSQL("EXISTS (SELECT 1 FROM stars s "+
"WHERE s.project_id = projects.id AND s.member_id = ?)")).
Needs("viewer").Filterable(),
NotNull().Needs("viewer").Filterable(),
).Expose(schema.REST{Ops: schema.CRUD | schema.OpList})
return r
}

// One declaration, and the field is in the row type with the expression beside
// it — the method rather than the tag, because SQL does not fit in a
// comma-separated list.
//
// The two derived fields also pin the nullability default from #147: a computed
// column is a pointer unless the declaration claims NotNull, because an
// expression can be NULL and there is no DDL to read the answer off.
func TestGeneratedModelCarriesTheExpression(t *testing.T) {
models := generate(t, computedFixture())["models_gen.go"]

for _, want := range []string{
`IsOverdue bool ` + "`" + `db:"is_overdue" json:"is_overdue" sqlb:"type:bool,filter,readonly"` + "`",
`IsOverdue *bool ` + "`" + `db:"is_overdue" json:"is_overdue" sqlb:"type:bool,filter,readonly"` + "`",
`IsStarred bool ` + "`" + `db:"is_starred" json:"is_starred" sqlb:"type:bool,filter,readonly"` + "`",
"func (Project) ComputedColumns() []sqlb.Computed {",
`{Name: "is_overdue", Expr: "due_date < current_date AND open_tasks > 0"},`,
`Needs: []string{"viewer"}`,
Expand Down
2 changes: 1 addition & 1 deletion codegen/override_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func TestOverriddenComputedColumnStillImportsSqlb(t *testing.T) {
schema.UUIDv7("id").PrimaryKey(),
schema.Int("open_tasks"),
schema.Computed("is_overdue", schema.TypeBool,
schema.FromSQL("open_tasks > 0")).Filterable(),
schema.FromSQL("open_tasks > 0")).NotNull().Filterable(),
)
// The override has to match the computed column, which is what puts it
// behind the guard. Matching by type is the narrowest way to say so.
Expand Down
12 changes: 8 additions & 4 deletions codegen/skill.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,16 @@ func skillResource(b *strings.Builder, t schema.TableManifest) {
b.WriteString("**Declared actions.** Domain verbs this resource owns. " +
"Reaching the same outcome by PATCHing a column is the mistake these exist to " +
"prevent — the verb owns the transition.\n\n")
b.WriteString("| Verb | Route | Writes |\n|---|---|---|\n")
b.WriteString("| Verb | Route | Writes | Also writes |\n|---|---|---|---|\n")
for _, a := range r.Actions {
fmt.Fprintf(b, "| `%s` | `%s %s` | %s |\n",
a.Name, a.Method, a.Path, orNone(joinCode(a.Writes, ", ")))
fmt.Fprintf(b, "| `%s` | `%s %s` | %s | %s |\n",
a.Name, a.Method, a.Path,
orNone(joinCode(a.Writes, ", ")), orNone(joinCode(a.Touches, ", ")))
}
b.WriteString("\n")
b.WriteString("\n*Writes* is the columns the envelope persists on the addressed row. " +
"*Also writes* is the tables the verb declares it reaches through its " +
"transaction — declared, not enforced, and *none* there means no claim was " +
"made rather than that none are written.\n\n")
}

if len(t.CollectedBy) > 0 {
Expand Down
21 changes: 21 additions & 0 deletions docs/adr/0041-computed-fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -301,3 +301,24 @@ reversible, a filter grammar is not.
made a computed column opt-in per reader, which removed the cost argument for
keeping derived work out of SQL. The taxonomy keeps four rows because a record
that quietly drops the option it rejected is not a record of the decision.

- 2026-08-05 — **Nullability inverted.** A computed column is now nullable
unless the declaration writes `NotNull()`, where a stored one stays not-null
unless it writes `Nullable()`. The record never stated a default, and the one
it inherited was the one an expression cannot honour: a correlated subquery
matching nothing is `NULL`, which for the reporting application was not an
edge case but every row with no project plus every row pointing at a deleted
one — a cross-module reference having no foreign key to prevent the second
([#147](https://github.com/jryannel/sqlb/issues/147)).

What made it expensive is where it landed. A stored column reads its
nullability off `NOT NULL` and the round trip checks it; a computed column has
no DDL, so `generate` had no opinion and `Diff` correctly ignored a column that
is not in the database. Both gates were green and the failure was a 500 at scan
time — `cannot scan NULL into *string`, naming the generated model rather than
the declaration that produced it — on data a fixture is unlikely to contain.

Inference over the expression was considered and rejected for the reason the
report gave: it is wrong in the unsafe direction wherever it is incomplete.
`NotNull()` is a claim rather than a check, and it fails the other way, which
is the direction this record already prefers everywhere else.
35 changes: 35 additions & 0 deletions docs/adr/0043-declared-actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -334,3 +334,38 @@ them is this record arguing with itself.
is load-bearing. `completeTask` writes a comment row through `sqlb.TxFrom`,
the `BeforeCreate` hook supplies its `workspace_id`, and a refused completion
rolls the comment back with it — none of which the declaration mentions.

- 2026-08-05 — **`Touches` added**, because the record's escape hatch turned out
to be invisible from every surface that describes a route
([#149](https://github.com/jryannel/sqlb/issues/149)). This record already
knew the shape of it — *"`completeTask` writes a comment row through
`sqlb.TxFrom` … none of which the declaration mentions"* is the last line of
the 2026-07-31 revision — and read it as a note about an example rather than
as a gap in the contract.

What made it one is that `Writes` is *reported* as complete, by three tools,
with nothing to say a verb can exceed it: `sqlb impact` states it, the OpenAPI
document carries it, and `--help` prints it. The CLI case is the sharp one,
because [ADR-0029](0029-go-cli.md)'s argument for the CLI is that `--help`
answers a caller with no compile step, "such as an agent" — and a declared
write set of two columns invites exactly the inference that the verb is
confined to one row. That inference can be wrong by ten tables, and the
repository already treats a plausible wrong answer as worse than a warning.

So `Touches []string` names tables beside `Writes`'s columns, and it is
documentation with no enforcement behind it. Enforcing it would mean tracing
application code the schema package cannot see; the alternative to an
unenforced claim was not an enforced one, it was silence. Validation refuses
only what says nothing — an empty name, a duplicate — and an unknown table is
accepted, since the cross-module write is the case the field exists for.

**Not a change to what an action covers.** The envelope stays declared and the
transition stays in Go; the eleven-table command is still a hand-written
command layer, and `Writes` naming columns on one row *should* understate it.
What changed is that the generated documentation can now tell the two apart.

The lock got the same treatment in prose rather than in code: the envelope's
`FOR UPDATE` covers the row it fetched, and statements issued through
`sqlb.TxFrom` take their own, in an order the application owns.
[docs/rest/actions.md](../rest/actions.md) says so under the `TxFrom` example,
which is where the reader is at the moment they are handed the transaction.
Loading
Loading