Skip to content

feat(graphile): presigned uploads only; drop graphile-llm metering - #1954

Open
pyramation wants to merge 5 commits into
mainfrom
feat/presigned-only-uploads-no-llm-metering
Open

pyramation wants to merge 5 commits into
mainfrom
feat/presigned-only-uploads-no-llm-metering

Conversation

@pyramation

@pyramation pyramation commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Uploads have one path. A file reaches a column through the presigned mutation (files row → PUT → ordinary update with the projection document). The streaming transport on the mutation itself is removed:

  • graphile-upload-plugin (the Upload scalar and the {column}Upload inputs) and graphile-settings/src/upload-resolver.ts are deleted; createConstructivePreset loses enableDirectUploads.
  • graphql/server no longer mounts graphql-upload or the content-type bridge, so a multipart/form-data POST to /graphql is refused (415). The request-size exemption for multipart goes with it.
  • graphile-presigned-url-plugin keeps buildFileProjection (now file-projection.ts); the staged-object helpers and the file-ref-registry lookup that only the streaming resolver used are gone.
  • express-context stops reading enable_direct_uploads.

Why: under Grafast a field's resolve() runs after its plan has executed, so the resolver wrapper swapped the upload in after the UPDATE had already run. A multipart updateX { xPatch: { photoUpload } } answered Attempted to update a record, but no new values were specified. and wrote nothing, while the schema kept advertising the input. A table with an upload-domain column also could not build at all: the domain's codec type Upload collided with the scalar.

graphile-llm drops its metering plugin (metering-plugin.ts, metering.ts, config-cache.ts, the metering preset option). It had the same resolve() problem, conflicted with LlmModulePlugin on build.llmEmbedder at schema build, and nothing enabled it. build.llmEmbedder now has one result shape, { embedding }; embedder failure messages no longer mention quotas or plans.

Tests

  • graphql/server-test/__tests__/upload-columns.integration.test.ts: real routed server (HTTP through grafserv, so documents are validated) over a table with image, upload and attachment columns — schema builds, inputs carry only the columns, no Upload scalar, a files row created through uploadAppFile is written to the column as its projection through the ordinary update, multipart is refused. On main the same fixture fails at schema build (Upload type conflict) and, without the upload column, advertises photoUpload and writes nothing.
  • graphile/graphile-llm/src/__tests__/preset-schema.test.ts: GraphileLlmPreset built into a real schema; the metering option is a type error.

Link to Devin session: https://app.devin.ai/sessions/edcf5248d26442ecab8f798ac6561247
Open in Devin Desktop: https://app.devin.ai/desktop/session/edcf5248d26442ecab8f798ac6561247?variant=devin
Requested by: @pyramation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@tenki-reviewer

tenki-reviewer Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Review complete. No blocking issues — approved ✅; 1 nitpick below.

🧹 Nitpicks (1) — 🟢 1 low
  • 🟢 Stale quota/plan wording in embedder failure messages (text-search-plugin.ts:92) — With metering removed from graphile-llm, the embedder returning null now means the provider is unavailable, not that a quota was exhausted — but user-facing errors still talk about billing: graphile/graphile-llm/src/plugins/text-search-plugin.ts:92 and line 121 say 'quota exceeded' and 'Upgrade your plan', and graphile/graphile-llm/src/plugins/text-mutation-plugin.ts:272-273 throws EMBED_QUOTA_EXCEEDED telling users to 'Upgrade your plan or wait for the next billing period'.

The change deletes the graphql-upload/multipart middleware path and the server-side managed upload module, replaces the removed constructiveUploadFieldDefinitions resolver with a standalone buildFileProjection, and strips metering/billing and the config cache from graphile-llm. Uploads now flow exclusively through the presigned-URL plugin, with the upload plugin reworked to a leaner plugin/preset/types surface. An integration test asserts multipart POSTs to /graphql now return 415, matching the intentional posture change in request-protection (multipart no longer exempt from the request-size cap). The enableDirectUploads flag is removed from preset options, types, and the database-settings loader, whose explicit select list tolerates old schemas still carrying the column.

Files Change
graphql/server, graphql/server-test Drop multipart-bridge/graphql-upload; request-protection caps multipart bodies and tests assert 415 on multipart /graphql
graphile/graphile-presigned-url-plugin Managed upload removed; buildFileProjection split into its own module with tests
graphile/graphile-settings, graphile/graphile-upload-plugin Upload-resolver and enableDirectUploads preset option deleted; upload plugin reworked and retested
graphile/graphile-llm Metering plugin, metering module, config cache, and public exports removed; preset schema test updated
packages/express-context, graphql/dev-server DatabaseSettings type and loader SQL drop enable_direct_uploads
CLAUDE.md, .github/workflows/run-tests.yaml, docs Package lists and CI coverage references updated for deleted packages

Reviewed commit: 8bb05fd

@devin-ai-integration
devin-ai-integration Bot force-pushed the feat/presigned-only-uploads-no-llm-metering branch from 3bcce2c to 055e670 Compare November 4, 2026 21:02
@blacksmith-sh

This comment has been minimized.

Uploads go through the presigned path only: the Upload scalar, the *Upload
mutation inputs, the graphql-upload middleware and the streaming resolver
are gone, and image/upload/attachment columns are plain columns on the
mutation inputs. graphile-llm no longer carries a metering plugin; the
embedder has one result shape.
@devin-ai-integration
devin-ai-integration Bot force-pushed the feat/presigned-only-uploads-no-llm-metering branch from 8228536 to 33bed39 Compare November 5, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant