Skip to content

feat: wire tablesMeta through pipeline, enrich M:N relations, add ORM add/remove junction methods#867

Open
pyramation wants to merge 4 commits intomainfrom
devin/1774052099-orm-m2m-methods
Open

feat: wire tablesMeta through pipeline, enrich M:N relations, add ORM add/remove junction methods#867
pyramation wants to merge 4 commits intomainfrom
devin/1774052099-orm-m2m-methods

Conversation

@pyramation
Copy link
Contributor

@pyramation pyramation commented Mar 21, 2026

Summary

PR 3 of the v5 relations feature. Consumes the tablesMeta plumbing added in PR 2 (#863) to enrich M:N relations with junction key field metadata, then generates add<Relation>/remove<Relation> ORM methods that delegate to the junction table's existing CRUD mutations.

4 files changed across 3 concerns:

  1. Pipeline wiring (pipeline/index.ts): Destructures tablesMeta from source.fetch() and calls the new enrichment step after table inference but before filtering.

  2. M:N enrichment (enrich-relations.ts — new): Matches tablesMeta entries to inferred tables by name, then populates junctionLeftKeyFields, junctionRightKeyFields, leftKeyFields, rightKeyFields on each ManyToManyRelation.

  3. ORM codegen (model-generator.ts, query-builder.ts): For each enriched M:N relation on a table, generates add<Singular> and remove<Singular> class methods. add calls buildCreateDocument against the junction table; remove calls a new buildJunctionRemoveDocument helper. Only emits methods when junction key metadata is present (graceful no-op for non-enriched relations). Uses the existing codegen utilities (getCreateMutationName, getDeleteMutationName, getCreateInputTypeName, getDeleteInputTypeName, getPrimaryKeyInfo, tsTypeFromPrimitive) and inflekt.singularize() for consistent, correct naming throughout.

Updates since last revision

  • Removed dead toPascalCase import that would fail lint
  • Removed async = true on add/remove methods to match existing CRUD method pattern
  • Replaced naive rel.fieldName.replace(/s$/, '') with inflekt.singularize() for correct handling of irregular plurals (e.g., categoriesaddCategory)
  • PK type resolution: Replaced hardcoded t.tsStringKeyword() with getPrimaryKeyInfo() + tsTypeFromPrimitive() to resolve actual PK types from left/right tables (e.g., integer PKs now correctly typed as number)
  • Consistent naming utilities: Replaced manual \Create${typeName}Input`string construction withgetCreateInputTypeName(), getDeleteInputTypeName(), getCreateMutationName(), getDeleteMutationName()` — hooks into the same inflector-based naming used by all other codegen
  • Conditional import: buildJunctionRemoveDocument is now only imported when at least one remove method will actually be generated (guards on junctionTable.query?.delete), preventing unused-import lint errors in generated output
  • Removed redundant leftTable lookup (table is already the left table)

Review & Testing Checklist for Human

  • { id: true } hardcoded as select in add method: The buildCreateDocument call passes { id: true } as the result selection. Junction tables with composite PKs and no id column would produce an invalid query. Verify junction tables always have an id field in the generated schema.
  • No new unit tests: Existing 317 tests pass, but no tests were added for enrichManyToManyRelations, buildJunctionRemoveDocument, or M:N method generation. Consider whether snapshot tests for generated model output with M:N relations are needed.
  • PK type fallback: When the right table isn't found in allTables (or a PK field name doesn't match), the type falls back to string. Verify this is acceptable for all expected junction table configurations.
  • Recommended test plan: Run pnpm generate or pnpm generate-orm against a project with provisioned M:N relations (e.g., project_tags junction between projects and tags). Inspect the generated model file to verify addTag/removeTag methods produce correct GraphQL documents with the right junction field mappings and correctly-typed parameters.

Notes

  • remove<Relation> is only generated when the junction table has a delete mutation (junctionTable.query?.delete is non-null). This is the correct guard — some junction tables may have delete disabled.
  • Enrichment is a no-op when tablesMeta is absent (e.g., endpoint/file sources that don't provide _meta), so this is fully backward compatible.
  • junctionDeleteMutation is computed with a getDeleteMutationName() fallback, but it's only used inside the if (junctionTable.query?.delete) guard where .query.delete is always truthy — so the fallback is effectively dead code. Harmless but could be tightened.

Link to Devin session: https://app.devin.ai/sessions/745d2d10b699452091e24131ba5edef2
Requested by: @pyramation


Open with Devin

… add/remove junction methods

- Destructure tablesMeta in runCodegenPipeline() alongside introspection
- Create enrich-relations.ts to populate ManyToManyRelation junction key fields from _meta
- Add buildJunctionRemoveDocument helper to query-builder template
- Generate add<Relation>/remove<Relation> methods on ORM models for M:N relations
- Methods delegate to junction table's existing create/delete CRUD mutations
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View 5 additional findings in Devin Review.

Open in Devin Review

{
// Parameters: one param per left PK + one param per right PK
const params: t.Identifier[] = [];
for (const lk of leftPkFields) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 All M:N method parameters hardcoded as string type regardless of actual PK type

In the add<Relation> and remove<Relation> method generation, all primary key parameters are unconditionally annotated with t.tsStringKeyword(). This produces incorrect TypeScript type annotations for tables with non-string primary keys (e.g., integer PKs typed as Int in GraphQL). A user calling addTag(123) would get a type error because the parameter expects string, and if they pass "123" instead, the GraphQL server would reject it because the mutation expects an Int. The actual PK types are available via the junction table's fields/constraints (accessible through allTables) but are not consulted.

Affected lines in add and remove methods

Lines 1022 and 1027 (add<Relation>):

p.typeAnnotation = t.tsTypeAnnotation(t.tsStringKeyword());

Lines 1091 and 1096 (remove<Relation>):

p.typeAnnotation = t.tsTypeAnnotation(t.tsStringKeyword());

For comparison, the existing findOne/update/delete methods correctly derive the PK type via tsTypeFromPrimitive(pkField.tsType) at model-generator.ts:277.

Prompt for agents
In graphql/codegen/src/core/codegen/orm/model-generator.ts, the M:N add/remove method parameters are all hardcoded as string type (tsStringKeyword). Instead, the actual PK types should be looked up from the junction table's fields or constraints. 

At minimum, for the left table you already have pkField/pkTsType available. For the right table, you could look up the right table from allTables and call getPrimaryKeyInfo() to get its PK types. Similarly for the junction key fields, you could look them up from junctionTable.fields.

Specifically, fix these four locations:
1. Line 1022: left PK params in add method - use the left table's actual PK type
2. Line 1027: right PK params in add method - look up right table PK type from allTables
3. Line 1091: left PK params in remove method - same as #1
4. Line 1096: right PK params in remove method - same as #2

Replace t.tsStringKeyword() with the appropriate type derived from tsTypeFromPrimitive() after looking up the actual field type.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

'buildFindFirstDocument',
'buildFindOneDocument',
'buildCreateDocument',
'buildUpdateByPkDocument',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 buildJunctionRemoveDocument imported based on M:N existence, not on whether remove methods are actually generated

At line 214, buildJunctionRemoveDocument is added to the import list whenever hasM2n is true (any M:N relation with junction key fields exists). However, the remove<Relation> method that actually uses this function is only generated when junctionDeleteMutation is defined (line 1087) AND the junction table is found in allTables (line 998-999). If neither condition is met for any M:N relation, the generated file will contain an unused import of buildJunctionRemoveDocument, which could cause lint errors in the generated output.

Prompt for agents
In graphql/codegen/src/core/codegen/orm/model-generator.ts, the decision to import buildJunctionRemoveDocument (line 214) is based on hasM2n alone, but the function is only used when junctionDeleteMutation is defined for at least one junction table. Either:
1. Move the import decision after the M:N method generation loop and track whether any remove methods were actually generated, OR
2. Pre-check whether any junction tables in allTables have query.delete defined before adding the import.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

… for M:N methods

- Use getCreateInputTypeName/getDeleteInputTypeName/getCreateMutationName/getDeleteMutationName instead of manual string construction
- Resolve actual PK types via getPrimaryKeyInfo() instead of hardcoding string
- Only import buildJunctionRemoveDocument when remove methods are actually generated
- Remove redundant leftTable lookup (table IS the left table)
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