Skip to content

refactor(showcase): drop the unreachable rowsOf normalizer from the nightly health sweep - #14841

Merged
os-sales merged 1 commit into
mainfrom
claude/issue-14460-drop-dead-rowsof
Sep 3, 2026
Merged

refactor(showcase): drop the unreachable rowsOf normalizer from the nightly health sweep#14841
os-sales merged 1 commit into
mainfrom
claude/issue-14460-drop-dead-rowsof

Conversation

@os-sales

@os-sales os-sales commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14460

Deletes the local rowsOf() normalizer from sweepProjectHealth and reads the two
ql.find(...) results as the arrays they are. Nothing else.

This is the repo's only shipped defineJob, so it is the copy-source for scheduled work.
A defensive shape that cannot fire teaches a false contract to whoever copies it.

The { records } limb is unreachable — measured on the tree, not assumed

rowsOf() accepted either an array or an object carrying a records array. The second
limb cannot fire for this handler. Four independent legs, all at e275b7b8:

  1. Declared engine contractpackages/spec/src/contracts/data-engine.ts:251:
    find(objectName, query?, options?) resolves to any[], an array.

  2. The implementation behind itpackages/objectql/src/engine.ts:9075,
    async find(object, query?, options?), also resolving to any[]. Its body has exactly
    two return statements: return hookContext.result and return opCtx.result as any[].
    Both carry the value of driver.find(...); every post-processing step between them is
    guarded by Array.isArray(result) and reassigns only arrays.

  3. The driver contract those two returns thread through
    packages/spec/src/contracts/data-driver.ts:155 declares find as resolving to an
    array of row objects. So the value the engine returns originates as an array by
    contract, not by convention.

  4. The one seam that could substitute a different value is unused. The
    return hookContext.result path returns whatever an afterFind hook left behind. The
    whole repo registers exactly one afterFind hook outside tests —
    packages/plugins/plugin-audit/src/read-audit.ts:600 (auditRead) — and that module
    contains no assignment to .result at all; it only reads it. The showcase app itself
    declares no afterFind hook: examples/app-showcase/src/data/hooks/index.ts declares
    only beforeInsert, beforeUpdate and afterUpdate.

And ctx.ql in this handler is that engine, not some other handle:
packages/runtime/src/job-handler-context.ts types JobHandlerContext.ql as
IObjectQLEngine; packages/runtime/src/app-plugin.ts:1029 builds the job context with
the ql resolved at app-plugin.ts:520 via ctx.getService('objectql'); and the
production registration of that service name is
packages/objectql/src/plugin.ts:400, ctx.registerService('objectql', this.ql), where
this.ql is new ObjectQL(...) — the class whose find is leg 2.

One more datum worth recording: the records envelope does exist in this repo, on a
different function with a different declared return —
packages/metadata/src/loaders/database-loader.ts, queryHistory(), a metadata-history
pagination shape. Nothing on ql.find's path produces it.

The two call sites, before and after

Reading note. In the "after" snippet the row annotation is written as the
placeholder ROW_ARRAY. In the file it is ordinary TypeScript generic syntax —
Array parameterised by Record of string to unknown. It is spelled as a
placeholder here because GitHub's body sanitizer deletes angle-bracket fragments from a
description: it ate that exact annotation out of the first version of this text, leaving
a bare Array> inside the code fence. Fences do not protect it. The literal bytes
are in the Files tab.

Before (the two rowsOf(...) wrappers):

  const projects = rowsOf(
    await ql.find('showcase_project', {
      where: { status: { $in: [...SWEPT_STATUSES] } },
      fields: ['id', 'status', 'health', 'budget', 'spent'],
      limit: READ_LIMIT,
      context: SYS,
    }),
  );
...
  const tasks = rowsOf(
    await ql.find('showcase_task', {
      where: { project: { $in: projectIds } },
      fields: ['project', 'progress'],
      limit: READ_LIMIT,
      context: SYS,
    }),
  );

After:

  const projects: ROW_ARRAY = await ql.find('showcase_project', {
    where: { status: { $in: [...SWEPT_STATUSES] } },
    fields: ['id', 'status', 'health', 'budget', 'spent'],
    limit: READ_LIMIT,
    context: SYS,
  });
...
  const tasks: ROW_ARRAY = await ql.find('showcase_task', {
    where: { project: { $in: projectIds } },
    fields: ['project', 'progress'],
    limit: READ_LIMIT,
    context: SYS,
  });

The explicit annotation is deliberate: the declared return is any[], and letting any
propagate would have silently weakened every downstream member read in this file
(project.budget, task.progress, p.id). The annotation keeps the exact static type
the code already had while deleting the runtime branch. No behaviour changes — the
normalizer was an identity on every value the engine can produce.

The delete leaves no dangling reference: rowsOf now occurs 0 times in the file (3
before), records 0 times, and the file's only import (JobHandlerContext) is still
used. Diff is 1 file, +12 / -23.

Tests

Suite covering this job: examples/app-showcase/test/inert-wirings.test.ts.

Measured both ways from the committed state, restoring the parent version of the file on
disk for the "before" leg and confirming each leg by blob hash before reading anything:

leg on-disk confirmation result
after (rowsOf deleted) rowsOf count 0 Tests 36 passed (36)
before (parent file restored) blob cd757ba8 == HEAD~1 blob, rowsOf count 3 Tests 36 passed (36)
restored blob 006c547c == HEAD blob, git diff HEAD empty

No rebuild leg was needed for either measurement: @objectstack/example-showcase exports
TypeScript source (main: ./objectstack.config.ts) and the suite imports the handler by
relative source path, so no dist/ sits between the edit and the assertion.

Both legs pass identically, which is the expected direction here rather than a red: the
suite's fake engine returns arrays on every path, so it never exercised the deleted limb.
That is corroboration that the limb was dead, and it means no coverage was lost.

Whole package, after: Test Files 27 passed (27), Tests 375 passed (375).
pnpm --filter @objectstack/example-showcase run typecheck (tsc --noEmit) clean —
and verified to actually cover the edit: tsc --listFiles lists
sweep-project-health.ts among the 1589 files in the program.

Gates

Derived from the real change set with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack
(1 path vs merge base 2d40f914), then run at e275b7b8 with exit codes captured by
redirect, never across a pipe.

15 of 17 green, including check:react-page-adapter-contract, which reports
21 app-showcase page module(s) + 1 content/docs react-page sample(s) ... no find() result is tested for array-ness.

Two exited 3, which each gate's own verdict text declares is not a result:

  • check-test-completenessNothing was measured: this gate exited before parsing a single summary line.
  • check:dual-build-cjs-loadsPREREQUISITE NOT MET — this gate reads built output, and some package has no dist/. Its self-test half passed (93 cases). Recorded as NOT
    MEASURED, not as a pass and not as a red; CI runs it after a full build.

Also run: pnpm check:nul-bytes green (8070 files, no raw control bytes), and the
repo-wide pnpm lint (eslint . --no-inline-config) — exit 0, no findings. That is the
full scan, not a narrowed one.

Changeset

None, skip-changeset. @objectstack/example-showcase is "private": true, so this diff
publishes nothing from any released package — AGENTS.md's stated criterion for the label.
The direct precedent is 253da34c4, the examples-only commit that created this very
handler, which carried no changeset either.

Out of scope, deliberately

Per the triage ruling, this card is the instance and not the class:
scripts/check-react-page-adapter-contract.mjs and its population are untouched. A second
instance of the same dead-normalizer shape exists at
packages/cli/src/commands/secret/orphans.ts (its own local rowsOf, over
secretDriver.find(...)); it is reported back to the dispatching PM rather than fixed
here — different package, different lane.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

… health sweep

`sweepProjectHealth` wrapped both of its `ql.find(...)` reads in a local
`rowsOf()` that accepted either an array or a `{ records }` envelope. The
`{ records }` limb cannot fire: `IObjectQLEngine.find` is declared
`Promise<any[]>`, ObjectQL's implementation returns arrays on both of its
return paths, and `IDataDriver.find` — the value those paths thread through
— is itself declared `Promise<Record<string, unknown>[]>`.

This is the repo's only shipped `defineJob`, so it is the copy-source for
scheduled work. A defensive shape that cannot fire teaches a false contract
to whoever copies it. Both reads now read as the contract actually is.

No behaviour change: the normalizer was an identity on every value the
engine can produce.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@github-actions github-actions Bot added the size/s label Sep 3, 2026
@os-sales os-sales added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026 — with Claude
@os-sales
os-sales marked this pull request as ready for review September 3, 2026 05:17
@os-sales
os-sales enabled auto-merge September 3, 2026 05:17
@os-sales
os-sales added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit 7251bad Sep 3, 2026
41 checks passed
@os-sales
os-sales deleted the claude/issue-14460-drop-dead-rowsof branch September 3, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] app-showcase's job sweep still normalizes { records } off an engine find() the contract types as an array

2 participants