Skip to content

feat: required field option for non-null types#2151

Closed
Noroth wants to merge 8 commits intomainfrom
ludwig/eng-7490-protographic-add-annotation-to-required-fields
Closed

feat: required field option for non-null types#2151
Noroth wants to merge 8 commits intomainfrom
ludwig/eng-7490-protographic-add-annotation-to-required-fields

Conversation

@Noroth
Copy link
Copy Markdown
Contributor

@Noroth Noroth commented Aug 19, 2025

Summary by CodeRabbit

  • New Features

    • Optional per-field "is_required" annotations for non-null GraphQL fields, enabled via a new config flag; getter added to retrieve generated lock data after generation.
  • Tests

    • Added/updated descriptor fixtures and many snapshot expectations to validate required-field annotations, wrappers, lists, and deprecation coexistence; new directive/option tests.
  • Chores

    • Improved test utility loading and error reporting; example config bumped to include the new option.

Checklist

@Noroth Noroth requested a review from StarpTech as a code owner August 19, 2025 17:37
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 19, 2025

Walkthrough

Adds optional per-field (is_required) protobuf option for non-null GraphQL fields (controlled by includeRequiredAnnotations), implements structured custom-field option collection/formatting and header emission (descriptor import/extension), refactors indentation helpers, exposes getGeneratedLockData(), and adds descriptor fixtures plus tests.

Changes

Cohort / File(s) Summary
Core visitor implementation
protographic/src/sdl-to-proto-visitor.ts
Adds includeRequiredAnnotations?: boolean; introduces internal option types and collectors (OptionType, CustomFieldOption, FieldOption, addCustomFieldOption); refactors field option rendering to formatFieldOptions/formatCustomFieldOptions; emits (is_required) for non-null fields; tracks hasRequiredAnnotations/customMessageOptions; refactors indentation helpers (formatIndent, getIndent); adds `getGeneratedLockData(): ProtoLock
Descriptor fixtures
protographic/tests/google/protobuf/descriptor.proto, protographic/tests/google/protobuf/descriptor.json
Adds proto2 descriptor.proto test fixture and corresponding JSON descriptor to support importing/registering google.protobuf.FieldOptions extension used for (is_required) in tests.
SDL-to-proto tests & snapshots
protographic/tests/sdl-to-proto/*.test.ts, protographic/tests/sdl-to-proto/10-options.test.ts, protographic/tests/sdl-to-proto/12-directive.test.ts
Updates many inline proto snapshots to include import "google/protobuf/descriptor.proto";, declares extend google.protobuf.FieldOptions { bool is_required = 50000; }, and annotates applicable fields with [(is_required) = true]. Adds tests validating required-option emission and combined deprecated+required annotations.
Other SDL-to-proto test updates
protographic/tests/sdl-to-proto/01-basic-types.test.ts, .../02-complex-types.test.ts, .../03-interfaces-unions.test.ts, .../04-federation.test.ts, .../05-edge-cases.test.ts, .../09-comments.test.ts, .../11-lists.test.ts
Snapshot updates across multiple tests to reflect descriptor import, extension declaration, and per-field is_required annotations for non-null GraphQL fields.
Test utilities
protographic/tests/util.ts
Adds path import; loads descriptor.proto fixture alongside wrappers; wraps validateProtoDefinition in try/catch and logs console.error before rethrowing.
Router test data
router-tests/testenv/testdata/configWithGRPC.json
Updates embedded protoSchema to import descriptor.proto, declare is_required extension, annotate many fields with is_required, and bumps version.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ludwig/eng-7490-protographic-add-annotation-to-required-fields

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (8)
protographic/src/sdl-to-proto-visitor.ts (3)

101-118: Option type abstraction: avoid non-proto type "number"

Proto doesn’t have a generic number scalar. If you plan to add numeric custom options later, define concrete types (e.g., int32/int64/double/float) or drop NUMBER until you need a concrete one. Leaving "number" here risks emitting invalid proto if it’s ever used.

Apply this minimal future-proofing:

 enum OptionType {
   STRING = 'string',
-  NUMBER = 'number',
+  // Add concrete numeric types on demand, e.g.:
+  // INT32 = 'int32',
+  // INT64 = 'int64',
+  // DOUBLE = 'double',
+  // FLOAT = 'float',
   BOOLEAN = 'bool',
 }

1254-1277: Field option rendering for deprecated/required — implementation looks solid

  • Deprecation detection is reused correctly.
  • Required annotation check aligns with GraphQL non-null.
  • formatFieldOptions handles custom vs. standard options and quoting.

Minor: Option emission order depends on push order; if determinism across future additions matters, consider sorting by option name before formatting. Not required now.

Also applies to: 1291-1301, 1314-1339


1-1: Prettier is failing on this file

CI reports a Prettier issue. Please run your formatter.

Recommended:

  • pnpm format (or) npx prettier --write protographic/src/sdl-to-proto-visitor.ts
protographic/tests/util.ts (3)

3-3: Missing semicolon and formatting

Add the missing semicolon to satisfy Prettier/lint.

-import * as path from 'path'
+import * as path from 'path';

12-29: Robustness: __dirname in ESM environments

If tests run under ESM, __dirname is undefined. Vitest commonly transpiles to CJS, but if you ever flip to ESM, consider this guard.

You can make this ESM-safe:

import * as path from 'path';
import { fileURLToPath } from 'url';

const __filename = typeof __dirname === 'undefined' ? fileURLToPath(import.meta.url) : __filename;
const __dirnameSafe = typeof __dirname === 'undefined' ? path.dirname(__filename) : __dirname;

root.loadSync(path.join(__dirnameSafe, 'google/protobuf/descriptor.proto'));

Not strictly required now, but it will save time if module settings change.


1-1: Prettier is failing on this file

CI reports a Prettier issue. Please run your formatter.

Recommended:

  • pnpm format (or) npx prettier --write protographic/tests/util.ts
protographic/tests/sdl-to-proto/12-directive.test.ts (1)

1-1: Prettier is failing on this file

CI reports a Prettier issue. Please run your formatter.

Recommended:

  • pnpm format (or) npx prettier --write protographic/tests/sdl-to-proto/12-directive.test.ts
protographic/tests/google/protobuf/descriptor.json (1)

1-1382: Consider generating this descriptor fixture at test time to avoid drift and repo bloat

This is a very large, version-sensitive artifact. Keeping it checked in risks drift vs. descriptor.proto and increases review noise. Prefer generating from descriptor.proto (or fetching from a pinned upstream source) during tests.

Example approach (TypeScript, using protobufjs) outside this file:

import { load } from "protobufjs";
import { join } from "node:path";

export async function loadDescriptorRoot() {
  // Use the checked-in .proto for determinism
  const protoPath = join(__dirname, "google/protobuf/descriptor.proto");
  const root = await load(protoPath);
  return root; // root.toJSON() if JSON reflection is required
}

If JSON is required by consumers, consider:

  • Minimizing to only the messages/enums actually used by tests.
  • Compressing the JSON fixture (e.g., .json.gz) and decompressing on test startup.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1ae6242 and 3a8f6d4.

📒 Files selected for processing (6)
  • protographic/src/sdl-to-proto-visitor.ts (12 hunks)
  • protographic/tests/google/protobuf/descriptor.json (1 hunks)
  • protographic/tests/google/protobuf/descriptor.proto (1 hunks)
  • protographic/tests/sdl-to-proto/10-options.test.ts (1 hunks)
  • protographic/tests/sdl-to-proto/12-directive.test.ts (1 hunks)
  • protographic/tests/util.ts (2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
protographic/tests/util.ts (1)
composition/scripts/get-composition-version.mjs (1)
  • __dirname (6-6)
🪛 GitHub Actions: Protographic CI
protographic/tests/util.ts

[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.

protographic/tests/sdl-to-proto/12-directive.test.ts

[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.

protographic/src/sdl-to-proto-visitor.ts

[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.

protographic/tests/google/protobuf/descriptor.json

[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.

🔇 Additional comments (8)
protographic/src/sdl-to-proto-visitor.ts (5)

89-91: Public option includeRequiredAnnotations added — looks good

The option is clearly named and documented; threading it through the visitor is sensible.


177-182: Private flags and accumulators wired in — OK

The internal flags and collections are appropriate for the new functionality.


195-211: Threading includeRequiredAnnotations through the ctor — OK

Default remains false; behavior gated appropriately.


1443-1466: Input fields: required and deprecated options — good parity with object fields

Logic mirrors object fields and maintains consistency across message kinds. Wrapper usage continues to be handled elsewhere.

Also applies to: 1480-1485


1896-1901: Indent helper refactor — OK

The dedicated formatIndent helper reads better than inline string multiplication. All usages here look correct.

Also applies to: 1906-1912

protographic/tests/google/protobuf/descriptor.proto (1)

11-11: Invalid enum assignment style: optimize_for should not be a string

The canonical descriptor uses option optimize_for = SPEED; (enum), not a string literal.

Apply this correction:

-option optimize_for = "SPEED";
+option optimize_for = SPEED;

Also, since this is a vendored descriptor for tests, consider aligning the entire file with the upstream canonical version to avoid subtle parser differences across tools. I can help replace this with an official copy if desired.

protographic/tests/sdl-to-proto/12-directive.test.ts (1)

501-538: Update snapshot to expect FieldOptions extension (not MessageOptions)

With the generator fix, the header should declare extend google.protobuf.FieldOptions, since (is_required) is used on fields.

Apply this snapshot change:

-      extend google.protobuf.MessageOptions {
+      extend google.protobuf.FieldOptions {
         optional bool is_required = 50000;
       }

After fixing the generator, re-run the tests to update inline snapshots if you prefer automatic updates.

protographic/tests/sdl-to-proto/10-options.test.ts (1)

78-151: Update snapshots to expect FieldOptions extension

The generator should declare custom field options on google.protobuf.FieldOptions. Update both snapshots accordingly.

Patch both snapshot sections:

-      extend google.protobuf.MessageOptions {
+      extend google.protobuf.FieldOptions {
         optional bool is_required = 50000;
       }

Run tests with snapshot update after applying the generator fix.

Also applies to: 153-191

Comment thread protographic/src/sdl-to-proto-visitor.ts Outdated
Comment thread protographic/src/sdl-to-proto-visitor.ts
Comment thread protographic/tests/google/protobuf/descriptor.json Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
protographic/src/sdl-to-proto-visitor.ts (1)

591-604: Custom option correctly targets FieldOptions (fix matches prior feedback)

The extension targets google.protobuf.FieldOptions and uses the reserved 50k–99,999 range. This resolves the earlier mismatch where MessageOptions was used while applying options on fields.

🧹 Nitpick comments (7)
protographic/src/sdl-to-proto-visitor.ts (6)

1268-1281: Only mark hasRequiredAnnotations when a required field is actually emitted

Currently hasRequiredAnnotations is set to true whenever includeRequiredAnnotations is enabled, even if no non-null fields exist. This causes descriptor import/extension emission in files that don’t actually use the custom option. Set it only when a required option is appended.

Apply this diff:

       if (required) {
         fieldOptions.push({
           name: 'is_required',
           value: 'true',
           valueType: OptionType.BOOLEAN,
           isCustom: true,
         });
+        this.hasRequiredAnnotations = true;
       }
-
-      this.hasRequiredAnnotations = true;

1449-1460: Same issue for input types: gate hasRequiredAnnotations on actual required usage

Mirror the fix in processInputObjectType so we don’t emit descriptor plumbing when all input fields are nullable.

       if (required) {
         fieldOptions.push({
           name: 'is_required',
           value: 'true',
           valueType: OptionType.BOOLEAN,
           isCustom: true,
         });
+        this.hasRequiredAnnotations = true;
       }
-      this.hasRequiredAnnotations = true;

221-227: go_package default never used due to guard; align code with comment

The comment says “Generate default go_package if not provided”, but the guard only runs when goPackage is provided. This means the default is never applied. Simplify and always set an option with fallback.

-// Initialize options
-if (goPackage && goPackage !== '') {
-  // Generate default go_package if not provided
-  const defaultGoPackage = `cosmo/pkg/proto/${packageName};${packageName.replace('.', '')}`;
-  const goPackageOption = goPackage || defaultGoPackage;
-  this.options.push(`option go_package = "${goPackageOption}";`);
-}
+// Initialize options (use provided value or sensible default)
+const defaultGoPackage = `cosmo/pkg/proto/${packageName};${packageName.replace('.', '')}`;
+const goPackageOption = goPackage && goPackage !== '' ? goPackage : defaultGoPackage;
+this.options.push(`option go_package = "${goPackageOption}";`);

If you intentionally don’t want a default, update the comment accordingly and keep the guard. Otherwise, the above aligns behavior with the docstring.


576-581: Prevent duplicate registration of custom options across multiple visits

If a visitor instance is reused and visit() is called multiple times, the same option could be registered multiple times, yielding duplicate extend fields with incremented numbers. Guard by name.

   private addCustomFieldOption(option: CustomFieldOption): void {
-    this.customMessageOptions.push(option);
+    if (!this.customMessageOptions.some((o) => o.name === option.name)) {
+      this.customMessageOptions.push(option);
+    }
   }

101-106: OptionType.NUMBER is not a valid Proto type literal

Using 'number' would produce invalid proto definitions if ever used. Either remove NUMBER until needed, or replace with concrete proto scalars (e.g., int32, int64, double) and introduce separate enum members.


177-185: Naming nit: customMessageOptions now represent FieldOptions

The property and related helpers are field-level, not message-level. Consider renaming customMessageOptions → customFieldOptions to avoid confusion.

protographic/tests/sdl-to-proto/09-comments.test.ts (1)

75-113: Consider adding a negative test: no required fields should not emit descriptor/extension

Add a case with at least one object/input type where all fields are nullable and verify that neither the descriptor import nor the extend block is present. This would guard against accidental emission and regressions around hasRequiredAnnotations.

I can add such a test case mirroring this file’s structure. Want me to draft it?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3a8f6d4 and 3ea8782.

📒 Files selected for processing (12)
  • protographic/src/sdl-to-proto-visitor.ts (12 hunks)
  • protographic/tests/google/protobuf/descriptor.json (1 hunks)
  • protographic/tests/sdl-to-proto/01-basic-types.test.ts (10 hunks)
  • protographic/tests/sdl-to-proto/02-complex-types.test.ts (10 hunks)
  • protographic/tests/sdl-to-proto/03-interfaces-unions.test.ts (8 hunks)
  • protographic/tests/sdl-to-proto/04-federation.test.ts (16 hunks)
  • protographic/tests/sdl-to-proto/05-edge-cases.test.ts (7 hunks)
  • protographic/tests/sdl-to-proto/09-comments.test.ts (8 hunks)
  • protographic/tests/sdl-to-proto/10-options.test.ts (1 hunks)
  • protographic/tests/sdl-to-proto/11-lists.test.ts (37 hunks)
  • protographic/tests/sdl-to-proto/12-directive.test.ts (13 hunks)
  • protographic/tests/util.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • protographic/tests/util.ts
  • protographic/tests/google/protobuf/descriptor.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T10:38:49.165Z
Learnt from: Noroth
PR: wundergraph/cosmo#2151
File: protographic/src/sdl-to-proto-visitor.ts:0-0
Timestamp: 2025-08-20T10:38:49.165Z
Learning: Inline `extend google.protobuf.FieldOptions` declarations in proto3 files work correctly with protoc, despite theoretical concerns about proto3 compatibility with extension declarations.

Applied to files:

  • protographic/tests/sdl-to-proto/05-edge-cases.test.ts
  • protographic/tests/sdl-to-proto/03-interfaces-unions.test.ts
  • protographic/tests/sdl-to-proto/09-comments.test.ts
  • protographic/tests/sdl-to-proto/11-lists.test.ts
  • protographic/tests/sdl-to-proto/12-directive.test.ts
  • protographic/src/sdl-to-proto-visitor.ts
🧬 Code Graph Analysis (3)
protographic/tests/sdl-to-proto/12-directive.test.ts (2)
protographic/src/index.ts (1)
  • compileGraphQLToProto (53-79)
protographic/tests/util.ts (1)
  • expectValidProto (36-38)
protographic/tests/sdl-to-proto/10-options.test.ts (2)
protographic/src/index.ts (1)
  • compileGraphQLToProto (53-79)
protographic/tests/util.ts (1)
  • expectValidProto (36-38)
protographic/src/sdl-to-proto-visitor.ts (1)
protographic/src/proto-lock.ts (1)
  • ProtoLockManager (23-193)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
🔇 Additional comments (19)
protographic/src/sdl-to-proto-visitor.ts (4)

496-503: Header plumbing looks correct; import descriptor only when needed

Conditionally adding descriptor.proto and registering the custom option only when includeRequiredAnnotations && hasRequiredAnnotations is True is the right call. Once the hasRequiredAnnotations gating is fixed (see earlier comment), this avoids emitting unused custom option scaffolding.


1319-1342: Field options formatter: solid, handles custom and built-ins cleanly

The formatter correctly quotes string values and wraps custom options with parentheses, yielding well-formed clauses like [(is_required) = true, deprecated = true].


1904-1910: Indentation helper usage improves readability

Switching to formatIndent here reduces manual spacing and keeps output consistent.


1938-1940: Public accessor for generated lock data is useful

The getGeneratedLockData() API addition is straightforward and aligns with the existing ProtoLockManager usage.

protographic/tests/sdl-to-proto/02-complex-types.test.ts (2)

34-39: Snapshot updates for descriptor import and FieldOptions extension look correct

The test now expects import "google/protobuf/descriptor.proto" and the FieldOptions extension block, matching the generator’s behavior when required annotations are present.


61-64: Verifies per-field (is_required) = true on non-null fields

Good coverage asserting annotations on both scalar and enum fields in messages.

protographic/tests/sdl-to-proto/09-comments.test.ts (3)

85-91: Comment-focused test also validates custom option header

The descriptor import and FieldOptions extension in the snapshot are consistent with the production change and do not interfere with comment formatting.


144-174: Ensures required annotations and comment blocks coexist cleanly

Asserting [(is_required) = true] alongside single- and multi-line comments is valuable; formatting remains correct.


507-527: Federation entity snapshots: required annotations correctly applied to key fields

The required annotations on id/name/upc/price are appropriate and consistent with the SDL’s non-nullability.

protographic/tests/sdl-to-proto/05-edge-cases.test.ts (3)

110-116: Header expectations for descriptor import and FieldOptions extension are aligned

For cases with required fields elsewhere in the file, expecting the descriptor/extension here is appropriate.


150-157: Correctly asserts (is_required) on reserved-name types

Good to see coverage on types that could conflict with Proto keywords; annotations are still rendered correctly.


584-613: Comprehensive required annotations across a complex graph

The breadth of assertions here is excellent (users, posts, comments, inputs, nested wrappers). This meaningfully exercises the generator.

protographic/tests/sdl-to-proto/03-interfaces-unions.test.ts (2)

40-45: Interface/union snapshots include proper descriptor import and option extension

Matches the generator’s new behavior in mixed polymorphic schemas.


76-85: Required annotations on interface implementors and union members look right

Asserting is_required across multiple shapes (User/Product/Article) helps ensure consistency.

protographic/tests/sdl-to-proto/04-federation.test.ts (1)

41-46: LGTM! Required field annotations correctly added.

The addition of the is_required field option infrastructure is properly implemented with the descriptor import and field extension. The non-null GraphQL fields are correctly annotated with [(is_required) = true] throughout the federation tests.

protographic/tests/sdl-to-proto/11-lists.test.ts (1)

28-33: LGTM! Required field annotations properly integrated.

The test correctly includes the descriptor import and field options extension. The is_required annotations are appropriately applied to non-null list fields throughout the test cases.

protographic/tests/sdl-to-proto/01-basic-types.test.ts (1)

197-203: LGTM! Required field annotations consistently applied.

The descriptor import and field options extension are properly added, and the is_required annotations correctly mark non-null fields like id and name in the User message.

protographic/tests/sdl-to-proto/10-options.test.ts (1)

148-186: LGTM! This test properly demonstrates required annotations.

The second test correctly shows the is_required extension and annotations on non-null input fields (id: ID!, name: String!). This validates the feature works as intended for input types.

protographic/tests/sdl-to-proto/12-directive.test.ts (1)

565-602: Excellent test for combined annotations!

This test effectively demonstrates that both deprecated and is_required annotations can be applied simultaneously to a field. The ordering [deprecated = true, (is_required) = true] on line 598 shows proper field option combination.

Comment thread protographic/tests/sdl-to-proto/10-options.test.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
protographic/tests/sdl-to-proto/10-options.test.ts (1)

78-100: Good: Scenario now exercises non-null fields and operation-field exclusion (addresses prior feedback).

Using non-null query fields plus a non-null mutation input/output directly validates the intended behavior and resolves the earlier review pointing out only-nullable SDL. The test name matches the semantics shown in the snapshot.

🧹 Nitpick comments (4)
protographic/tests/sdl-to-proto/10-options.test.ts (4)

102-104: Add a focused test to assert header/extension gating when the feature is enabled but unused.

The PR claims conditional emission of the descriptor import/extension. Add a case where includeRequiredAnnotations: true is set but there are no non-null input/object fields, and assert that descriptor.proto and the extend block are not emitted, and no (is_required) appears.

Example test you can drop into this file:

it('does not emit descriptor import/extension when includeRequiredAnnotations is true but unused', () => {
  const sdl = `
    type Query {
      name: String
    }
  `;

  const { proto: protoText } = compileGraphQLToProto(sdl, {
    includeRequiredAnnotations: true,
  });

  expectValidProto(protoText);
  expect(protoText).not.toContain('google/protobuf/descriptor.proto');
  expect(protoText).not.toContain('extend google.protobuf.FieldOptions');
  expect(protoText).not.toMatch(/\(is_required\)/);
});

106-108: Reduce snapshot brittleness with a few targeted semantic assertions.

Add explicit checks to ensure: (1) no is_required on operation responses, (2) non-null scalars in operations are not wrapped, (3) is_required is present on non-null fields in message types.

Apply this diff:

@@
-    expectValidProto(protoText);
+    expectValidProto(protoText);
+
+    // Sanity: header and custom option are present
+    expect(protoText).toContain('extend google.protobuf.FieldOptions');
+    // Required annotations should not appear on operation response fields
+    expect(protoText).not.toMatch(/message QueryStringFieldResponse[\s\S]*is_required/);
+    expect(protoText).not.toMatch(/message QueryIntFieldResponse[\s\S]*is_required/);
+    // Non-null scalars in operations use plain types (no wrappers)
+    expect(protoText).toMatch(/message QueryStringFieldResponse[\s\S]*\n\s*string string_field = 1;/);
+    expect(protoText).toMatch(/message QueryIntFieldResponse[\s\S]*\n\s*int32 int_field = 1;/);
+    // Input/Object messages annotate non-null fields
+    expect(protoText).toMatch(/message UserInput[\s\S]*name = 1 \[\(is_required\) = true\];/);
+    expect(protoText).toMatch(/message User[\s\S]*id = 1 \[\(is_required\) = true\];/);

184-184: Nit: Align test name wording (“option” vs “options”).

Other tests use plural “options”. Consider renaming for consistency.

Apply this diff:

-it('should generate proto with required option for input types', () => {
+it('should generate proto with required options for input types', () => {

198-200: Add explicit asserts for annotations and wrappers in the input-only scenario.

These checks make the test resilient while keeping the snapshot for broader shape.

Apply this diff:

@@
-    expectValidProto(protoText);
+    expectValidProto(protoText);
+
+    // Header present and custom option usage
+    expect(protoText).toContain('extend google.protobuf.FieldOptions');
+    // Non-null fields annotated; nullable use wrappers
+    expect(protoText).toMatch(/message UserInput[\s\S]*id = 1 \[\(is_required\) = true\];/);
+    expect(protoText).toMatch(/message UserInput[\s\S]*name = 2 \[\(is_required\) = true\];/);
+    expect(protoText).toMatch(/google\.protobuf\.StringValue email = 3;/);
+    expect(protoText).toMatch(/google\.protobuf\.Int32Value age = 4;/);
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3ea8782 and 36d68e8.

📒 Files selected for processing (1)
  • protographic/tests/sdl-to-proto/10-options.test.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
protographic/tests/sdl-to-proto/10-options.test.ts (2)
protographic/src/index.ts (1)
  • compileGraphQLToProto (53-79)
protographic/tests/util.ts (1)
  • expectValidProto (36-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
protographic/tests/sdl-to-proto/10-options.test.ts (3)

173-180: Correct application of is_required on message fields.

Annotating only message fields (UserInput.name, User.id/name) and not operation request/response fields matches the intended contract.


204-210: Header emission looks correct when required annotations are in use.

Importing descriptor.proto and declaring the is_required extension only when used is the right trade-off.


112-118: No changes needed for optional in FieldOptions extension
The optional label inside extend google.protobuf.FieldOptions is valid in a proto3 file and is widely supported by protoc (including older versions) and popular code generators. It simply follows the proto2-style extension semantics for FieldOptions and does not enable proto3’s new presence-tracking behavior—so you can safely keep it.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 20, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-632d668f65954b078ff43debd1e969b4558c82fd

@Noroth Noroth requested review from a team as code owners August 20, 2025 12:13
@Noroth Noroth requested a review from JivusAyrus August 20, 2025 12:13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
router-tests/testenv/testdata/configWithGRPC.json (1)

1796-1797: Optional: consider list-of-nullables semantics for [X]! cases

For GraphQL types like [Task]! (non-null list of nullable items), proto repeated Task cannot represent null elements inside the list. If you need to exercise null elements in tests, consider using a wrapper element (e.g., a oneof container) or a wrapper list to allow explicit null markers. If not required by tests, current mapping is fine.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 36d68e8 and 9b00ee9.

⛔ Files ignored due to path filters (2)
  • demo/pkg/subgraphs/projects/generated/service.pb.go is excluded by !**/*.pb.go, !**/generated/**
  • demo/pkg/subgraphs/projects/generated/service.proto is excluded by !**/generated/**
📒 Files selected for processing (1)
  • router-tests/testenv/testdata/configWithGRPC.json (2 hunks)
🔇 Additional comments (3)
router-tests/testenv/testdata/configWithGRPC.json (3)

1796-1797: Overall: solid coverage of is_required annotations and descriptor import

Good job importing google/protobuf/descriptor.proto, defining a high-numbered custom option, and consistently annotating non-null fields (including inputs). The wrappers vs. repeated choices align with prior patterns in this test suite.


2202-2202: Version bump acknowledged

The config version UUID was updated. No issues.


1796-1797: No conflicts: is_required = 50000 is isolated per‐schema and supported by protoc

All occurrences of the extend google.protobuf.FieldOptions { optional bool is_required = 50000; } you found live only inside:

  • Inline protoSchema snippets in individual JSON test configs (configWithGRPC.json, configWithPlugins.json)
  • The generated demo/pkg/subgraphs/projects/generated/service.proto (with matching Go extension in service.pb.go)
  • Dozens of standalone protographic test cases, each embedding its own schema

Each of these is compiled in isolation—there are no multiple extension definitions in a single protoc invocation—and every schema imports google/protobuf/descriptor.proto. Protoc (and grpc-tools) includes descriptor.proto by default, and proto3 accepts custom extensions without extra flags. No duplicate‐tag collisions or unsupported syntax remain.

No action required.

Comment on lines 1796 to 1797
"protoSchema": "syntax = \"proto3\";\npackage service;\n\noption go_package = \"github.com/wundergraph/cosmo/demo/pkg/subgraphs/projects\";\n\nimport \"google/protobuf/descriptor.proto\";\nimport \"google/protobuf/wrappers.proto\";\n\nextend google.protobuf.FieldOptions {\n optional bool is_required = 50000;\n}\n\n// Service definition for ProjectsService\nservice ProjectsService {\n // Lookup Employee entity by id\n rpc LookupEmployeeById(LookupEmployeeByIdRequest) returns (LookupEmployeeByIdResponse) {}\n // Lookup Milestone entity by id\n rpc LookupMilestoneById(LookupMilestoneByIdRequest) returns (LookupMilestoneByIdResponse) {}\n // Lookup Product entity by upc\n rpc LookupProductByUpc(LookupProductByUpcRequest) returns (LookupProductByUpcResponse) {}\n // Lookup Project entity by id\n rpc LookupProjectById(LookupProjectByIdRequest) returns (LookupProjectByIdResponse) {}\n // Lookup Task entity by id\n rpc LookupTaskById(LookupTaskByIdRequest) returns (LookupTaskByIdResponse) {}\n rpc MutationAddMilestone(MutationAddMilestoneRequest) returns (MutationAddMilestoneResponse) {}\n rpc MutationAddProject(MutationAddProjectRequest) returns (MutationAddProjectResponse) {}\n rpc MutationAddTask(MutationAddTaskRequest) returns (MutationAddTaskResponse) {}\n rpc MutationUpdateProjectStatus(MutationUpdateProjectStatusRequest) returns (MutationUpdateProjectStatusResponse) {}\n rpc QueryArchivedProjects(QueryArchivedProjectsRequest) returns (QueryArchivedProjectsResponse) {}\n rpc QueryKillService(QueryKillServiceRequest) returns (QueryKillServiceResponse) {}\n rpc QueryMilestones(QueryMilestonesRequest) returns (QueryMilestonesResponse) {}\n rpc QueryNodesById(QueryNodesByIdRequest) returns (QueryNodesByIdResponse) {}\n rpc QueryPanic(QueryPanicRequest) returns (QueryPanicResponse) {}\n rpc QueryProject(QueryProjectRequest) returns (QueryProjectResponse) {}\n rpc QueryProjectActivities(QueryProjectActivitiesRequest) returns (QueryProjectActivitiesResponse) {}\n rpc QueryProjectResources(QueryProjectResourcesRequest) returns (QueryProjectResourcesResponse) {}\n rpc QueryProjectStatuses(QueryProjectStatusesRequest) returns (QueryProjectStatusesResponse) {}\n rpc QueryProjectTags(QueryProjectTagsRequest) returns (QueryProjectTagsResponse) {}\n rpc QueryProjects(QueryProjectsRequest) returns (QueryProjectsResponse) {}\n rpc QueryProjectsByStatus(QueryProjectsByStatusRequest) returns (QueryProjectsByStatusResponse) {}\n rpc QueryResourceMatrix(QueryResourceMatrixRequest) returns (QueryResourceMatrixResponse) {}\n rpc QuerySearchProjects(QuerySearchProjectsRequest) returns (QuerySearchProjectsResponse) {}\n rpc QueryTasks(QueryTasksRequest) returns (QueryTasksResponse) {}\n rpc QueryTasksByPriority(QueryTasksByPriorityRequest) returns (QueryTasksByPriorityResponse) {}\n}\n\n// Wrapper message for a list of Employee.\nmessage ListOfEmployee {\n message List {\n repeated Employee items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Int.\nmessage ListOfInt {\n message List {\n repeated int32 items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfListOfListOfTask {\n message List {\n repeated ListOfListOfTask items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Milestone.\nmessage ListOfListOfMilestone {\n message List {\n repeated ListOfMilestone items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Project.\nmessage ListOfListOfProject {\n message List {\n repeated ListOfProject items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of ProjectResource.\nmessage ListOfListOfProjectResource {\n message List {\n repeated ListOfProjectResource items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of String.\nmessage ListOfListOfString {\n message List {\n repeated ListOfString items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfListOfTask {\n message List {\n repeated ListOfTask items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Milestone.\nmessage ListOfMilestone {\n message List {\n repeated Milestone items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Project.\nmessage ListOfProject {\n message List {\n repeated Project items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of ProjectResource.\nmessage ListOfProjectResource {\n message List {\n repeated ProjectResource items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of String.\nmessage ListOfString {\n message List {\n repeated string items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfTask {\n message List {\n repeated Task items = 1;\n }\n List list = 1;\n}\n// Key message for Project entity lookup\nmessage LookupProjectByIdRequestKey {\n // Key field for Project entity lookup.\n string id = 1;\n}\n\n// Request message for Project entity lookup.\nmessage LookupProjectByIdRequest {\n /*\n * List of keys to look up Project entities.\n * Order matters - each key maps to one entity in LookupProjectByIdResponse.\n */\n repeated LookupProjectByIdRequestKey keys = 1;\n}\n\n// Response message for Project entity lookup.\nmessage LookupProjectByIdResponse {\n /*\n * List of Project entities in the same order as the keys in LookupProjectByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Project result = 1;\n}\n\n// Key message for Milestone entity lookup\nmessage LookupMilestoneByIdRequestKey {\n // Key field for Milestone entity lookup.\n string id = 1;\n}\n\n// Request message for Milestone entity lookup.\nmessage LookupMilestoneByIdRequest {\n /*\n * List of keys to look up Milestone entities.\n * Order matters - each key maps to one entity in LookupMilestoneByIdResponse.\n */\n repeated LookupMilestoneByIdRequestKey keys = 1;\n}\n\n// Response message for Milestone entity lookup.\nmessage LookupMilestoneByIdResponse {\n /*\n * List of Milestone entities in the same order as the keys in LookupMilestoneByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Milestone result = 1;\n}\n\n// Key message for Task entity lookup\nmessage LookupTaskByIdRequestKey {\n // Key field for Task entity lookup.\n string id = 1;\n}\n\n// Request message for Task entity lookup.\nmessage LookupTaskByIdRequest {\n /*\n * List of keys to look up Task entities.\n * Order matters - each key maps to one entity in LookupTaskByIdResponse.\n */\n repeated LookupTaskByIdRequestKey keys = 1;\n}\n\n// Response message for Task entity lookup.\nmessage LookupTaskByIdResponse {\n /*\n * List of Task entities in the same order as the keys in LookupTaskByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Task result = 1;\n}\n\n// Key message for Employee entity lookup\nmessage LookupEmployeeByIdRequestKey {\n // Key field for Employee entity lookup.\n string id = 1;\n}\n\n// Request message for Employee entity lookup.\nmessage LookupEmployeeByIdRequest {\n /*\n * List of keys to look up Employee entities.\n * Order matters - each key maps to one entity in LookupEmployeeByIdResponse.\n */\n repeated LookupEmployeeByIdRequestKey keys = 1;\n}\n\n// Response message for Employee entity lookup.\nmessage LookupEmployeeByIdResponse {\n /*\n * List of Employee entities in the same order as the keys in LookupEmployeeByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Employee result = 1;\n}\n\n// Key message for Product entity lookup\nmessage LookupProductByUpcRequestKey {\n // Key field for Product entity lookup.\n string upc = 1;\n}\n\n// Request message for Product entity lookup.\nmessage LookupProductByUpcRequest {\n /*\n * List of keys to look up Product entities.\n * Order matters - each key maps to one entity in LookupProductByUpcResponse.\n */\n repeated LookupProductByUpcRequestKey keys = 1;\n}\n\n// Response message for Product entity lookup.\nmessage LookupProductByUpcResponse {\n /*\n * List of Product entities in the same order as the keys in LookupProductByUpcRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Product result = 1;\n}\n\n// Request message for projects operation.\nmessage QueryProjectsRequest {\n}\n// Response message for projects operation.\nmessage QueryProjectsResponse {\n repeated Project projects = 1;\n}\n// Request message for project operation.\nmessage QueryProjectRequest {\n string id = 1;\n}\n// Response message for project operation.\nmessage QueryProjectResponse {\n Project project = 1;\n}\n// Request message for projectStatuses operation.\nmessage QueryProjectStatusesRequest {\n}\n// Response message for projectStatuses operation.\nmessage QueryProjectStatusesResponse {\n repeated ProjectStatus project_statuses = 1;\n}\n// Request message for projectsByStatus operation.\nmessage QueryProjectsByStatusRequest {\n ProjectStatus status = 1;\n}\n// Response message for projectsByStatus operation.\nmessage QueryProjectsByStatusResponse {\n repeated Project projects_by_status = 1;\n}\n// Request message for projectResources operation.\nmessage QueryProjectResourcesRequest {\n string project_id = 1;\n}\n// Response message for projectResources operation.\nmessage QueryProjectResourcesResponse {\n repeated ProjectResource project_resources = 1;\n}\n// Request message for searchProjects operation.\nmessage QuerySearchProjectsRequest {\n string query = 1;\n}\n// Response message for searchProjects operation.\nmessage QuerySearchProjectsResponse {\n repeated ProjectSearchResult search_projects = 1;\n}\n// Request message for milestones operation.\nmessage QueryMilestonesRequest {\n string project_id = 1;\n}\n// Response message for milestones operation.\nmessage QueryMilestonesResponse {\n repeated Milestone milestones = 1;\n}\n// Request message for tasks operation.\nmessage QueryTasksRequest {\n string project_id = 1;\n}\n// Response message for tasks operation.\nmessage QueryTasksResponse {\n repeated Task tasks = 1;\n}\n// Request message for projectActivities operation.\nmessage QueryProjectActivitiesRequest {\n string project_id = 1;\n}\n// Response message for projectActivities operation.\nmessage QueryProjectActivitiesResponse {\n repeated ProjectActivity project_activities = 1;\n}\n// Request message for projectTags operation.\nmessage QueryProjectTagsRequest {\n}\n// Response message for projectTags operation.\nmessage QueryProjectTagsResponse {\n ListOfString project_tags = 1;\n}\n// Request message for archivedProjects operation.\nmessage QueryArchivedProjectsRequest {\n}\n// Response message for archivedProjects operation.\nmessage QueryArchivedProjectsResponse {\n repeated Project archived_projects = 1;\n}\n// Request message for tasksByPriority operation.\nmessage QueryTasksByPriorityRequest {\n string project_id = 1;\n}\n// Response message for tasksByPriority operation.\nmessage QueryTasksByPriorityResponse {\n ListOfListOfTask tasks_by_priority = 1;\n}\n// Request message for resourceMatrix operation.\nmessage QueryResourceMatrixRequest {\n string project_id = 1;\n}\n// Response message for resourceMatrix operation.\nmessage QueryResourceMatrixResponse {\n ListOfListOfProjectResource resource_matrix = 1;\n}\n// Request message for killService operation.\nmessage QueryKillServiceRequest {\n}\n// Response message for killService operation.\nmessage QueryKillServiceResponse {\n bool kill_service = 1;\n}\n// Request message for panic operation.\nmessage QueryPanicRequest {\n}\n// Response message for panic operation.\nmessage QueryPanicResponse {\n bool panic = 1;\n}\n// Request message for nodesById operation.\nmessage QueryNodesByIdRequest {\n string id = 1;\n}\n// Response message for nodesById operation.\nmessage QueryNodesByIdResponse {\n repeated Node nodes_by_id = 1;\n}\n// Request message for addProject operation.\nmessage MutationAddProjectRequest {\n ProjectInput project = 1;\n}\n// Response message for addProject operation.\nmessage MutationAddProjectResponse {\n Project add_project = 1;\n}\n// Request message for addMilestone operation.\nmessage MutationAddMilestoneRequest {\n MilestoneInput milestone = 1;\n}\n// Response message for addMilestone operation.\nmessage MutationAddMilestoneResponse {\n Milestone add_milestone = 1;\n}\n// Request message for addTask operation.\nmessage MutationAddTaskRequest {\n TaskInput task = 1;\n}\n// Response message for addTask operation.\nmessage MutationAddTaskResponse {\n Task add_task = 1;\n}\n// Request message for updateProjectStatus operation.\nmessage MutationUpdateProjectStatusRequest {\n string project_id = 1;\n ProjectStatus status = 2;\n}\n// Response message for updateProjectStatus operation.\nmessage MutationUpdateProjectStatusResponse {\n ProjectUpdate update_project_status = 1;\n}\n\nmessage Project {\n string id = 1 [(is_required) = true];\n string name = 2 [(is_required) = true];\n google.protobuf.StringValue description = 3;\n google.protobuf.StringValue start_date = 4;\n google.protobuf.StringValue end_date = 5;\n ProjectStatus status = 6 [(is_required) = true];\n repeated Employee team_members = 7 [(is_required) = true];\n repeated Product related_products = 8 [(is_required) = true];\n ListOfString milestone_ids = 9;\n repeated Milestone milestones = 10 [(is_required) = true];\n repeated Task tasks = 11 [(is_required) = true];\n google.protobuf.DoubleValue progress = 12;\n ListOfString tags = 13;\n ListOfProject alternative_projects = 14;\n ListOfProject dependencies = 15;\n ListOfListOfProjectResource resource_groups = 16 [(is_required) = true];\n ListOfListOfTask tasks_by_phase = 17 [(is_required) = true];\n ListOfListOfMilestone milestone_groups = 18;\n ListOfListOfListOfTask priority_matrix = 19;\n}\n\nmessage Milestone {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n string name = 3 [(is_required) = true];\n google.protobuf.StringValue description = 4;\n google.protobuf.StringValue start_date = 5;\n google.protobuf.StringValue end_date = 6;\n MilestoneStatus status = 7 [(is_required) = true];\n google.protobuf.DoubleValue completion_percentage = 8;\n repeated Milestone dependencies = 9 [(is_required) = true];\n ListOfTask subtasks = 10;\n ListOfEmployee reviewers = 11;\n}\n\nmessage Task {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n google.protobuf.StringValue milestone_id = 3;\n google.protobuf.Int32Value assignee_id = 4;\n string name = 5 [(is_required) = true];\n google.protobuf.StringValue description = 6;\n TaskPriority priority = 7 [(is_required) = true];\n TaskStatus status = 8 [(is_required) = true];\n // Deprecation notice: No more estimations!\n google.protobuf.DoubleValue estimated_hours = 9 [deprecated = true];\n google.protobuf.DoubleValue actual_hours = 10;\n google.protobuf.StringValue created_at = 11;\n google.protobuf.StringValue completed_at = 12;\n ListOfString labels = 13;\n ListOfTask subtasks = 14;\n repeated Task dependencies = 15 [(is_required) = true];\n repeated string attachment_urls = 16 [(is_required) = true];\n ListOfInt reviewer_ids = 17;\n}\n\nmessage Employee {\n int32 id = 1 [(is_required) = true];\n ListOfProject projects = 2;\n repeated Task assigned_tasks = 3 [(is_required) = true];\n repeated Task completed_tasks = 4 [(is_required) = true];\n ListOfString skills = 5;\n ListOfString certifications = 6;\n ListOfListOfProject project_history = 7 [(is_required) = true];\n}\n\nmessage Product {\n string upc = 1 [(is_required) = true];\n ListOfProject projects = 2;\n ListOfListOfString feature_matrix = 3;\n}\n\nenum ProjectStatus {\n PROJECT_STATUS_UNSPECIFIED = 0;\n PROJECT_STATUS_PLANNING = 1;\n PROJECT_STATUS_ACTIVE = 2;\n PROJECT_STATUS_COMPLETED = 3;\n PROJECT_STATUS_ON_HOLD = 4;\n}\n\nmessage ProjectResource {\n oneof value {\n Employee employee = 1;\n Product product = 2;\n Milestone milestone = 3;\n Task task = 4;\n }\n}\n\nmessage ProjectSearchResult {\n oneof value {\n Project project = 1;\n Milestone milestone = 2;\n Task task = 3;\n }\n}\n\nmessage ProjectActivity {\n oneof value {\n ProjectUpdate project_update = 1;\n Milestone milestone = 2;\n Task task = 3;\n }\n}\n\nmessage Node {\n oneof instance {\n Project project = 1;\n Milestone milestone = 2;\n Task task = 3;\n ProjectUpdate project_update = 4;\n }\n}\n\nmessage ProjectInput {\n string name = 1 [(is_required) = true];\n google.protobuf.StringValue description = 2;\n google.protobuf.StringValue start_date = 3;\n google.protobuf.StringValue end_date = 4;\n ProjectStatus status = 5 [(is_required) = true];\n}\n\nmessage MilestoneInput {\n string project_id = 1 [(is_required) = true];\n string name = 2 [(is_required) = true];\n google.protobuf.StringValue description = 3;\n google.protobuf.StringValue due_date = 4;\n MilestoneStatus status = 5 [(is_required) = true];\n}\n\nmessage TaskInput {\n string project_id = 1 [(is_required) = true];\n google.protobuf.Int32Value assignee_id = 2;\n string name = 3 [(is_required) = true];\n google.protobuf.StringValue description = 4;\n TaskPriority priority = 5 [(is_required) = true];\n TaskStatus status = 6 [(is_required) = true];\n google.protobuf.DoubleValue estimated_hours = 7;\n}\n\nmessage ProjectUpdate {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n int32 updated_by_id = 3 [(is_required) = true];\n ProjectUpdateType update_type = 4 [(is_required) = true];\n string description = 5 [(is_required) = true];\n string timestamp = 6 [(is_required) = true];\n google.protobuf.StringValue metadata = 7;\n}\n\nmessage Timestamped {\n oneof instance {\n Project project = 1;\n Milestone milestone = 2;\n }\n}\n\nmessage Assignable {\n oneof instance {\n Task task = 1;\n }\n}\n\nenum MilestoneStatus {\n MILESTONE_STATUS_UNSPECIFIED = 0;\n MILESTONE_STATUS_PENDING = 1;\n MILESTONE_STATUS_IN_PROGRESS = 2;\n MILESTONE_STATUS_COMPLETED = 3;\n MILESTONE_STATUS_DELAYED = 4;\n}\n\nenum TaskStatus {\n TASK_STATUS_UNSPECIFIED = 0;\n TASK_STATUS_TODO = 1;\n TASK_STATUS_IN_PROGRESS = 2;\n TASK_STATUS_REVIEW = 3;\n TASK_STATUS_COMPLETED = 4;\n TASK_STATUS_BLOCKED = 5;\n}\n\nenum TaskPriority {\n TASK_PRIORITY_UNSPECIFIED = 0;\n TASK_PRIORITY_LOW = 1;\n TASK_PRIORITY_MEDIUM = 2;\n TASK_PRIORITY_HIGH = 3;\n TASK_PRIORITY_URGENT = 4;\n}\n\nenum ProjectUpdateType {\n PROJECT_UPDATE_TYPE_UNSPECIFIED = 0;\n PROJECT_UPDATE_TYPE_STATUS_CHANGE = 1;\n PROJECT_UPDATE_TYPE_MILESTONE_ADDED = 2;\n PROJECT_UPDATE_TYPE_TASK_ASSIGNED = 3;\n PROJECT_UPDATE_TYPE_PROGRESS_UPDATE = 4;\n PROJECT_UPDATE_TYPE_TEAM_CHANGE = 5;\n}"
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missed (is_required) on Project.priority_matrix (outer list is non-null in GraphQL)

The GraphQL SDL defines priorityMatrix: [[[Task!]!]!] (outer list non-null). The proto field currently lacks the (is_required) = true option, which makes it inconsistent with the intended non-nullability.

Adjust the field in the protoSchema string:

-  ListOfListOfListOfTask priority_matrix = 19;
+  ListOfListOfListOfTask priority_matrix = 19 [(is_required) = true];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"protoSchema": "syntax = \"proto3\";\npackage service;\n\noption go_package = \"github.com/wundergraph/cosmo/demo/pkg/subgraphs/projects\";\n\nimport \"google/protobuf/descriptor.proto\";\nimport \"google/protobuf/wrappers.proto\";\n\nextend google.protobuf.FieldOptions {\n optional bool is_required = 50000;\n}\n\n// Service definition for ProjectsService\nservice ProjectsService {\n // Lookup Employee entity by id\n rpc LookupEmployeeById(LookupEmployeeByIdRequest) returns (LookupEmployeeByIdResponse) {}\n // Lookup Milestone entity by id\n rpc LookupMilestoneById(LookupMilestoneByIdRequest) returns (LookupMilestoneByIdResponse) {}\n // Lookup Product entity by upc\n rpc LookupProductByUpc(LookupProductByUpcRequest) returns (LookupProductByUpcResponse) {}\n // Lookup Project entity by id\n rpc LookupProjectById(LookupProjectByIdRequest) returns (LookupProjectByIdResponse) {}\n // Lookup Task entity by id\n rpc LookupTaskById(LookupTaskByIdRequest) returns (LookupTaskByIdResponse) {}\n rpc MutationAddMilestone(MutationAddMilestoneRequest) returns (MutationAddMilestoneResponse) {}\n rpc MutationAddProject(MutationAddProjectRequest) returns (MutationAddProjectResponse) {}\n rpc MutationAddTask(MutationAddTaskRequest) returns (MutationAddTaskResponse) {}\n rpc MutationUpdateProjectStatus(MutationUpdateProjectStatusRequest) returns (MutationUpdateProjectStatusResponse) {}\n rpc QueryArchivedProjects(QueryArchivedProjectsRequest) returns (QueryArchivedProjectsResponse) {}\n rpc QueryKillService(QueryKillServiceRequest) returns (QueryKillServiceResponse) {}\n rpc QueryMilestones(QueryMilestonesRequest) returns (QueryMilestonesResponse) {}\n rpc QueryNodesById(QueryNodesByIdRequest) returns (QueryNodesByIdResponse) {}\n rpc QueryPanic(QueryPanicRequest) returns (QueryPanicResponse) {}\n rpc QueryProject(QueryProjectRequest) returns (QueryProjectResponse) {}\n rpc QueryProjectActivities(QueryProjectActivitiesRequest) returns (QueryProjectActivitiesResponse) {}\n rpc QueryProjectResources(QueryProjectResourcesRequest) returns (QueryProjectResourcesResponse) {}\n rpc QueryProjectStatuses(QueryProjectStatusesRequest) returns (QueryProjectStatusesResponse) {}\n rpc QueryProjectTags(QueryProjectTagsRequest) returns (QueryProjectTagsResponse) {}\n rpc QueryProjects(QueryProjectsRequest) returns (QueryProjectsResponse) {}\n rpc QueryProjectsByStatus(QueryProjectsByStatusRequest) returns (QueryProjectsByStatusResponse) {}\n rpc QueryResourceMatrix(QueryResourceMatrixRequest) returns (QueryResourceMatrixResponse) {}\n rpc QuerySearchProjects(QuerySearchProjectsRequest) returns (QuerySearchProjectsResponse) {}\n rpc QueryTasks(QueryTasksRequest) returns (QueryTasksResponse) {}\n rpc QueryTasksByPriority(QueryTasksByPriorityRequest) returns (QueryTasksByPriorityResponse) {}\n}\n\n// Wrapper message for a list of Employee.\nmessage ListOfEmployee {\n message List {\n repeated Employee items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Int.\nmessage ListOfInt {\n message List {\n repeated int32 items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfListOfListOfTask {\n message List {\n repeated ListOfListOfTask items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Milestone.\nmessage ListOfListOfMilestone {\n message List {\n repeated ListOfMilestone items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Project.\nmessage ListOfListOfProject {\n message List {\n repeated ListOfProject items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of ProjectResource.\nmessage ListOfListOfProjectResource {\n message List {\n repeated ListOfProjectResource items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of String.\nmessage ListOfListOfString {\n message List {\n repeated ListOfString items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfListOfTask {\n message List {\n repeated ListOfTask items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Milestone.\nmessage ListOfMilestone {\n message List {\n repeated Milestone items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Project.\nmessage ListOfProject {\n message List {\n repeated Project items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of ProjectResource.\nmessage ListOfProjectResource {\n message List {\n repeated ProjectResource items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of String.\nmessage ListOfString {\n message List {\n repeated string items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfTask {\n message List {\n repeated Task items = 1;\n }\n List list = 1;\n}\n// Key message for Project entity lookup\nmessage LookupProjectByIdRequestKey {\n // Key field for Project entity lookup.\n string id = 1;\n}\n\n// Request message for Project entity lookup.\nmessage LookupProjectByIdRequest {\n /*\n * List of keys to look up Project entities.\n * Order matters - each key maps to one entity in LookupProjectByIdResponse.\n */\n repeated LookupProjectByIdRequestKey keys = 1;\n}\n\n// Response message for Project entity lookup.\nmessage LookupProjectByIdResponse {\n /*\n * List of Project entities in the same order as the keys in LookupProjectByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Project result = 1;\n}\n\n// Key message for Milestone entity lookup\nmessage LookupMilestoneByIdRequestKey {\n // Key field for Milestone entity lookup.\n string id = 1;\n}\n\n// Request message for Milestone entity lookup.\nmessage LookupMilestoneByIdRequest {\n /*\n * List of keys to look up Milestone entities.\n * Order matters - each key maps to one entity in LookupMilestoneByIdResponse.\n */\n repeated LookupMilestoneByIdRequestKey keys = 1;\n}\n\n// Response message for Milestone entity lookup.\nmessage LookupMilestoneByIdResponse {\n /*\n * List of Milestone entities in the same order as the keys in LookupMilestoneByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Milestone result = 1;\n}\n\n// Key message for Task entity lookup\nmessage LookupTaskByIdRequestKey {\n // Key field for Task entity lookup.\n string id = 1;\n}\n\n// Request message for Task entity lookup.\nmessage LookupTaskByIdRequest {\n /*\n * List of keys to look up Task entities.\n * Order matters - each key maps to one entity in LookupTaskByIdResponse.\n */\n repeated LookupTaskByIdRequestKey keys = 1;\n}\n\n// Response message for Task entity lookup.\nmessage LookupTaskByIdResponse {\n /*\n * List of Task entities in the same order as the keys in LookupTaskByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Task result = 1;\n}\n\n// Key message for Employee entity lookup\nmessage LookupEmployeeByIdRequestKey {\n // Key field for Employee entity lookup.\n string id = 1;\n}\n\n// Request message for Employee entity lookup.\nmessage LookupEmployeeByIdRequest {\n /*\n * List of keys to look up Employee entities.\n * Order matters - each key maps to one entity in LookupEmployeeByIdResponse.\n */\n repeated LookupEmployeeByIdRequestKey keys = 1;\n}\n\n// Response message for Employee entity lookup.\nmessage LookupEmployeeByIdResponse {\n /*\n * List of Employee entities in the same order as the keys in LookupEmployeeByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Employee result = 1;\n}\n\n// Key message for Product entity lookup\nmessage LookupProductByUpcRequestKey {\n // Key field for Product entity lookup.\n string upc = 1;\n}\n\n// Request message for Product entity lookup.\nmessage LookupProductByUpcRequest {\n /*\n * List of keys to look up Product entities.\n * Order matters - each key maps to one entity in LookupProductByUpcResponse.\n */\n repeated LookupProductByUpcRequestKey keys = 1;\n}\n\n// Response message for Product entity lookup.\nmessage LookupProductByUpcResponse {\n /*\n * List of Product entities in the same order as the keys in LookupProductByUpcRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Product result = 1;\n}\n\n// Request message for projects operation.\nmessage QueryProjectsRequest {\n}\n// Response message for projects operation.\nmessage QueryProjectsResponse {\n repeated Project projects = 1;\n}\n// Request message for project operation.\nmessage QueryProjectRequest {\n string id = 1;\n}\n// Response message for project operation.\nmessage QueryProjectResponse {\n Project project = 1;\n}\n// Request message for projectStatuses operation.\nmessage QueryProjectStatusesRequest {\n}\n// Response message for projectStatuses operation.\nmessage QueryProjectStatusesResponse {\n repeated ProjectStatus project_statuses = 1;\n}\n// Request message for projectsByStatus operation.\nmessage QueryProjectsByStatusRequest {\n ProjectStatus status = 1;\n}\n// Response message for projectsByStatus operation.\nmessage QueryProjectsByStatusResponse {\n repeated Project projects_by_status = 1;\n}\n// Request message for projectResources operation.\nmessage QueryProjectResourcesRequest {\n string project_id = 1;\n}\n// Response message for projectResources operation.\nmessage QueryProjectResourcesResponse {\n repeated ProjectResource project_resources = 1;\n}\n// Request message for searchProjects operation.\nmessage QuerySearchProjectsRequest {\n string query = 1;\n}\n// Response message for searchProjects operation.\nmessage QuerySearchProjectsResponse {\n repeated ProjectSearchResult search_projects = 1;\n}\n// Request message for milestones operation.\nmessage QueryMilestonesRequest {\n string project_id = 1;\n}\n// Response message for milestones operation.\nmessage QueryMilestonesResponse {\n repeated Milestone milestones = 1;\n}\n// Request message for tasks operation.\nmessage QueryTasksRequest {\n string project_id = 1;\n}\n// Response message for tasks operation.\nmessage QueryTasksResponse {\n repeated Task tasks = 1;\n}\n// Request message for projectActivities operation.\nmessage QueryProjectActivitiesRequest {\n string project_id = 1;\n}\n// Response message for projectActivities operation.\nmessage QueryProjectActivitiesResponse {\n repeated ProjectActivity project_activities = 1;\n}\n// Request message for projectTags operation.\nmessage QueryProjectTagsRequest {\n}\n// Response message for projectTags operation.\nmessage QueryProjectTagsResponse {\n ListOfString project_tags = 1;\n}\n// Request message for archivedProjects operation.\nmessage QueryArchivedProjectsRequest {\n}\n// Response message for archivedProjects operation.\nmessage QueryArchivedProjectsResponse {\n repeated Project archived_projects = 1;\n}\n// Request message for tasksByPriority operation.\nmessage QueryTasksByPriorityRequest {\n string project_id = 1;\n}\n// Response message for tasksByPriority operation.\nmessage QueryTasksByPriorityResponse {\n ListOfListOfTask tasks_by_priority = 1;\n}\n// Request message for resourceMatrix operation.\nmessage QueryResourceMatrixRequest {\n string project_id = 1;\n}\n// Response message for resourceMatrix operation.\nmessage QueryResourceMatrixResponse {\n ListOfListOfProjectResource resource_matrix = 1;\n}\n// Request message for killService operation.\nmessage QueryKillServiceRequest {\n}\n// Response message for killService operation.\nmessage QueryKillServiceResponse {\n bool kill_service = 1;\n}\n// Request message for panic operation.\nmessage QueryPanicRequest {\n}\n// Response message for panic operation.\nmessage QueryPanicResponse {\n bool panic = 1;\n}\n// Request message for nodesById operation.\nmessage QueryNodesByIdRequest {\n string id = 1;\n}\n// Response message for nodesById operation.\nmessage QueryNodesByIdResponse {\n repeated Node nodes_by_id = 1;\n}\n// Request message for addProject operation.\nmessage MutationAddProjectRequest {\n ProjectInput project = 1;\n}\n// Response message for addProject operation.\nmessage MutationAddProjectResponse {\n Project add_project = 1;\n}\n// Request message for addMilestone operation.\nmessage MutationAddMilestoneRequest {\n MilestoneInput milestone = 1;\n}\n// Response message for addMilestone operation.\nmessage MutationAddMilestoneResponse {\n Milestone add_milestone = 1;\n}\n// Request message for addTask operation.\nmessage MutationAddTaskRequest {\n TaskInput task = 1;\n}\n// Response message for addTask operation.\nmessage MutationAddTaskResponse {\n Task add_task = 1;\n}\n// Request message for updateProjectStatus operation.\nmessage MutationUpdateProjectStatusRequest {\n string project_id = 1;\n ProjectStatus status = 2;\n}\n// Response message for updateProjectStatus operation.\nmessage MutationUpdateProjectStatusResponse {\n ProjectUpdate update_project_status = 1;\n}\n\nmessage Project {\n string id = 1 [(is_required) = true];\n string name = 2 [(is_required) = true];\n google.protobuf.StringValue description = 3;\n google.protobuf.StringValue start_date = 4;\n google.protobuf.StringValue end_date = 5;\n ProjectStatus status = 6 [(is_required) = true];\n repeated Employee team_members = 7 [(is_required) = true];\n repeated Product related_products = 8 [(is_required) = true];\n ListOfString milestone_ids = 9;\n repeated Milestone milestones = 10 [(is_required) = true];\n repeated Task tasks = 11 [(is_required) = true];\n google.protobuf.DoubleValue progress = 12;\n ListOfString tags = 13;\n ListOfProject alternative_projects = 14;\n ListOfProject dependencies = 15;\n ListOfListOfProjectResource resource_groups = 16 [(is_required) = true];\n ListOfListOfTask tasks_by_phase = 17 [(is_required) = true];\n ListOfListOfMilestone milestone_groups = 18;\n ListOfListOfListOfTask priority_matrix = 19;\n}\n\nmessage Milestone {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n string name = 3 [(is_required) = true];\n google.protobuf.StringValue description = 4;\n google.protobuf.StringValue start_date = 5;\n google.protobuf.StringValue end_date = 6;\n MilestoneStatus status = 7 [(is_required) = true];\n google.protobuf.DoubleValue completion_percentage = 8;\n repeated Milestone dependencies = 9 [(is_required) = true];\n ListOfTask subtasks = 10;\n ListOfEmployee reviewers = 11;\n}\n\nmessage Task {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n google.protobuf.StringValue milestone_id = 3;\n google.protobuf.Int32Value assignee_id = 4;\n string name = 5 [(is_required) = true];\n google.protobuf.StringValue description = 6;\n TaskPriority priority = 7 [(is_required) = true];\n TaskStatus status = 8 [(is_required) = true];\n // Deprecation notice: No more estimations!\n google.protobuf.DoubleValue estimated_hours = 9 [deprecated = true];\n google.protobuf.DoubleValue actual_hours = 10;\n google.protobuf.StringValue created_at = 11;\n google.protobuf.StringValue completed_at = 12;\n ListOfString labels = 13;\n ListOfTask subtasks = 14;\n repeated Task dependencies = 15 [(is_required) = true];\n repeated string attachment_urls = 16 [(is_required) = true];\n ListOfInt reviewer_ids = 17;\n}\n\nmessage Employee {\n int32 id = 1 [(is_required) = true];\n ListOfProject projects = 2;\n repeated Task assigned_tasks = 3 [(is_required) = true];\n repeated Task completed_tasks = 4 [(is_required) = true];\n ListOfString skills = 5;\n ListOfString certifications = 6;\n ListOfListOfProject project_history = 7 [(is_required) = true];\n}\n\nmessage Product {\n string upc = 1 [(is_required) = true];\n ListOfProject projects = 2;\n ListOfListOfString feature_matrix = 3;\n}\n\nenum ProjectStatus {\n PROJECT_STATUS_UNSPECIFIED = 0;\n PROJECT_STATUS_PLANNING = 1;\n PROJECT_STATUS_ACTIVE = 2;\n PROJECT_STATUS_COMPLETED = 3;\n PROJECT_STATUS_ON_HOLD = 4;\n}\n\nmessage ProjectResource {\n oneof value {\n Employee employee = 1;\n Product product = 2;\n Milestone milestone = 3;\n Task task = 4;\n }\n}\n\nmessage ProjectSearchResult {\n oneof value {\n Project project = 1;\n Milestone milestone = 2;\n Task task = 3;\n }\n}\n\nmessage ProjectActivity {\n oneof value {\n ProjectUpdate project_update = 1;\n Milestone milestone = 2;\n Task task = 3;\n }\n}\n\nmessage Node {\n oneof instance {\n Project project = 1;\n Milestone milestone = 2;\n Task task = 3;\n ProjectUpdate project_update = 4;\n }\n}\n\nmessage ProjectInput {\n string name = 1 [(is_required) = true];\n google.protobuf.StringValue description = 2;\n google.protobuf.StringValue start_date = 3;\n google.protobuf.StringValue end_date = 4;\n ProjectStatus status = 5 [(is_required) = true];\n}\n\nmessage MilestoneInput {\n string project_id = 1 [(is_required) = true];\n string name = 2 [(is_required) = true];\n google.protobuf.StringValue description = 3;\n google.protobuf.StringValue due_date = 4;\n MilestoneStatus status = 5 [(is_required) = true];\n}\n\nmessage TaskInput {\n string project_id = 1 [(is_required) = true];\n google.protobuf.Int32Value assignee_id = 2;\n string name = 3 [(is_required) = true];\n google.protobuf.StringValue description = 4;\n TaskPriority priority = 5 [(is_required) = true];\n TaskStatus status = 6 [(is_required) = true];\n google.protobuf.DoubleValue estimated_hours = 7;\n}\n\nmessage ProjectUpdate {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n int32 updated_by_id = 3 [(is_required) = true];\n ProjectUpdateType update_type = 4 [(is_required) = true];\n string description = 5 [(is_required) = true];\n string timestamp = 6 [(is_required) = true];\n google.protobuf.StringValue metadata = 7;\n}\n\nmessage Timestamped {\n oneof instance {\n Project project = 1;\n Milestone milestone = 2;\n }\n}\n\nmessage Assignable {\n oneof instance {\n Task task = 1;\n }\n}\n\nenum MilestoneStatus {\n MILESTONE_STATUS_UNSPECIFIED = 0;\n MILESTONE_STATUS_PENDING = 1;\n MILESTONE_STATUS_IN_PROGRESS = 2;\n MILESTONE_STATUS_COMPLETED = 3;\n MILESTONE_STATUS_DELAYED = 4;\n}\n\nenum TaskStatus {\n TASK_STATUS_UNSPECIFIED = 0;\n TASK_STATUS_TODO = 1;\n TASK_STATUS_IN_PROGRESS = 2;\n TASK_STATUS_REVIEW = 3;\n TASK_STATUS_COMPLETED = 4;\n TASK_STATUS_BLOCKED = 5;\n}\n\nenum TaskPriority {\n TASK_PRIORITY_UNSPECIFIED = 0;\n TASK_PRIORITY_LOW = 1;\n TASK_PRIORITY_MEDIUM = 2;\n TASK_PRIORITY_HIGH = 3;\n TASK_PRIORITY_URGENT = 4;\n}\n\nenum ProjectUpdateType {\n PROJECT_UPDATE_TYPE_UNSPECIFIED = 0;\n PROJECT_UPDATE_TYPE_STATUS_CHANGE = 1;\n PROJECT_UPDATE_TYPE_MILESTONE_ADDED = 2;\n PROJECT_UPDATE_TYPE_TASK_ASSIGNED = 3;\n PROJECT_UPDATE_TYPE_PROGRESS_UPDATE = 4;\n PROJECT_UPDATE_TYPE_TEAM_CHANGE = 5;\n}"
}
// … inside the protoSchema string …
message Project {
// … other fields …
ListOfListOfListOfTask priority_matrix = 19 [(is_required) = true];
// … following fields …
}
🤖 Prompt for AI Agents
In router-tests/testenv/testdata/configWithGRPC.json around lines 1796 to 1797,
the protoSchema string's Project message is missing the (is_required) = true
option on the priority_matrix field; update the Project definition inside the
protoSchema string to change the field declaration for priority_matrix from
"ListOfListOfListOfTask priority_matrix = 19;" to include the option, i.e. add
"[(is_required) = true]" to that field so it matches the GraphQL SDL outer-list
non-nullability.

🛠️ Refactor suggestion

Proto3 custom option: drop the optional label in the extend FieldOptions block

In proto3, custom options declared via extend google.protobuf.FieldOptions should not use the optional label. The canonical form is just bool is_required = 50000;. Keeping optional can break protoc parsing depending on version/flags.

Apply this diff inside the protoSchema string:

-extend google.protobuf.FieldOptions {
-  optional bool is_required = 50000;
-}
+extend google.protobuf.FieldOptions {
+  bool is_required = 50000;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"protoSchema": "syntax = \"proto3\";\npackage service;\n\noption go_package = \"github.com/wundergraph/cosmo/demo/pkg/subgraphs/projects\";\n\nimport \"google/protobuf/descriptor.proto\";\nimport \"google/protobuf/wrappers.proto\";\n\nextend google.protobuf.FieldOptions {\n optional bool is_required = 50000;\n}\n\n// Service definition for ProjectsService\nservice ProjectsService {\n // Lookup Employee entity by id\n rpc LookupEmployeeById(LookupEmployeeByIdRequest) returns (LookupEmployeeByIdResponse) {}\n // Lookup Milestone entity by id\n rpc LookupMilestoneById(LookupMilestoneByIdRequest) returns (LookupMilestoneByIdResponse) {}\n // Lookup Product entity by upc\n rpc LookupProductByUpc(LookupProductByUpcRequest) returns (LookupProductByUpcResponse) {}\n // Lookup Project entity by id\n rpc LookupProjectById(LookupProjectByIdRequest) returns (LookupProjectByIdResponse) {}\n // Lookup Task entity by id\n rpc LookupTaskById(LookupTaskByIdRequest) returns (LookupTaskByIdResponse) {}\n rpc MutationAddMilestone(MutationAddMilestoneRequest) returns (MutationAddMilestoneResponse) {}\n rpc MutationAddProject(MutationAddProjectRequest) returns (MutationAddProjectResponse) {}\n rpc MutationAddTask(MutationAddTaskRequest) returns (MutationAddTaskResponse) {}\n rpc MutationUpdateProjectStatus(MutationUpdateProjectStatusRequest) returns (MutationUpdateProjectStatusResponse) {}\n rpc QueryArchivedProjects(QueryArchivedProjectsRequest) returns (QueryArchivedProjectsResponse) {}\n rpc QueryKillService(QueryKillServiceRequest) returns (QueryKillServiceResponse) {}\n rpc QueryMilestones(QueryMilestonesRequest) returns (QueryMilestonesResponse) {}\n rpc QueryNodesById(QueryNodesByIdRequest) returns (QueryNodesByIdResponse) {}\n rpc QueryPanic(QueryPanicRequest) returns (QueryPanicResponse) {}\n rpc QueryProject(QueryProjectRequest) returns (QueryProjectResponse) {}\n rpc QueryProjectActivities(QueryProjectActivitiesRequest) returns (QueryProjectActivitiesResponse) {}\n rpc QueryProjectResources(QueryProjectResourcesRequest) returns (QueryProjectResourcesResponse) {}\n rpc QueryProjectStatuses(QueryProjectStatusesRequest) returns (QueryProjectStatusesResponse) {}\n rpc QueryProjectTags(QueryProjectTagsRequest) returns (QueryProjectTagsResponse) {}\n rpc QueryProjects(QueryProjectsRequest) returns (QueryProjectsResponse) {}\n rpc QueryProjectsByStatus(QueryProjectsByStatusRequest) returns (QueryProjectsByStatusResponse) {}\n rpc QueryResourceMatrix(QueryResourceMatrixRequest) returns (QueryResourceMatrixResponse) {}\n rpc QuerySearchProjects(QuerySearchProjectsRequest) returns (QuerySearchProjectsResponse) {}\n rpc QueryTasks(QueryTasksRequest) returns (QueryTasksResponse) {}\n rpc QueryTasksByPriority(QueryTasksByPriorityRequest) returns (QueryTasksByPriorityResponse) {}\n}\n\n// Wrapper message for a list of Employee.\nmessage ListOfEmployee {\n message List {\n repeated Employee items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Int.\nmessage ListOfInt {\n message List {\n repeated int32 items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfListOfListOfTask {\n message List {\n repeated ListOfListOfTask items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Milestone.\nmessage ListOfListOfMilestone {\n message List {\n repeated ListOfMilestone items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Project.\nmessage ListOfListOfProject {\n message List {\n repeated ListOfProject items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of ProjectResource.\nmessage ListOfListOfProjectResource {\n message List {\n repeated ListOfProjectResource items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of String.\nmessage ListOfListOfString {\n message List {\n repeated ListOfString items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfListOfTask {\n message List {\n repeated ListOfTask items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Milestone.\nmessage ListOfMilestone {\n message List {\n repeated Milestone items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Project.\nmessage ListOfProject {\n message List {\n repeated Project items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of ProjectResource.\nmessage ListOfProjectResource {\n message List {\n repeated ProjectResource items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of String.\nmessage ListOfString {\n message List {\n repeated string items = 1;\n }\n List list = 1;\n}\n// Wrapper message for a list of Task.\nmessage ListOfTask {\n message List {\n repeated Task items = 1;\n }\n List list = 1;\n}\n// Key message for Project entity lookup\nmessage LookupProjectByIdRequestKey {\n // Key field for Project entity lookup.\n string id = 1;\n}\n\n// Request message for Project entity lookup.\nmessage LookupProjectByIdRequest {\n /*\n * List of keys to look up Project entities.\n * Order matters - each key maps to one entity in LookupProjectByIdResponse.\n */\n repeated LookupProjectByIdRequestKey keys = 1;\n}\n\n// Response message for Project entity lookup.\nmessage LookupProjectByIdResponse {\n /*\n * List of Project entities in the same order as the keys in LookupProjectByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Project result = 1;\n}\n\n// Key message for Milestone entity lookup\nmessage LookupMilestoneByIdRequestKey {\n // Key field for Milestone entity lookup.\n string id = 1;\n}\n\n// Request message for Milestone entity lookup.\nmessage LookupMilestoneByIdRequest {\n /*\n * List of keys to look up Milestone entities.\n * Order matters - each key maps to one entity in LookupMilestoneByIdResponse.\n */\n repeated LookupMilestoneByIdRequestKey keys = 1;\n}\n\n// Response message for Milestone entity lookup.\nmessage LookupMilestoneByIdResponse {\n /*\n * List of Milestone entities in the same order as the keys in LookupMilestoneByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Milestone result = 1;\n}\n\n// Key message for Task entity lookup\nmessage LookupTaskByIdRequestKey {\n // Key field for Task entity lookup.\n string id = 1;\n}\n\n// Request message for Task entity lookup.\nmessage LookupTaskByIdRequest {\n /*\n * List of keys to look up Task entities.\n * Order matters - each key maps to one entity in LookupTaskByIdResponse.\n */\n repeated LookupTaskByIdRequestKey keys = 1;\n}\n\n// Response message for Task entity lookup.\nmessage LookupTaskByIdResponse {\n /*\n * List of Task entities in the same order as the keys in LookupTaskByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Task result = 1;\n}\n\n// Key message for Employee entity lookup\nmessage LookupEmployeeByIdRequestKey {\n // Key field for Employee entity lookup.\n string id = 1;\n}\n\n// Request message for Employee entity lookup.\nmessage LookupEmployeeByIdRequest {\n /*\n * List of keys to look up Employee entities.\n * Order matters - each key maps to one entity in LookupEmployeeByIdResponse.\n */\n repeated LookupEmployeeByIdRequestKey keys = 1;\n}\n\n// Response message for Employee entity lookup.\nmessage LookupEmployeeByIdResponse {\n /*\n * List of Employee entities in the same order as the keys in LookupEmployeeByIdRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Employee result = 1;\n}\n\n// Key message for Product entity lookup\nmessage LookupProductByUpcRequestKey {\n // Key field for Product entity lookup.\n string upc = 1;\n}\n\n// Request message for Product entity lookup.\nmessage LookupProductByUpcRequest {\n /*\n * List of keys to look up Product entities.\n * Order matters - each key maps to one entity in LookupProductByUpcResponse.\n */\n repeated LookupProductByUpcRequestKey keys = 1;\n}\n\n// Response message for Product entity lookup.\nmessage LookupProductByUpcResponse {\n /*\n * List of Product entities in the same order as the keys in LookupProductByUpcRequest.\n * Always return the same number of entities as keys. Use null for entities that cannot be found.\n * \n * Example:\n * LookupUserByIdRequest:\n * keys:\n * - id: 1\n * - id: 2\n * LookupUserByIdResponse:\n * result:\n * - id: 1 # User with id 1 found\n * - null # User with id 2 not found\n */\n repeated Product result = 1;\n}\n\n// Request message for projects operation.\nmessage QueryProjectsRequest {\n}\n// Response message for projects operation.\nmessage QueryProjectsResponse {\n repeated Project projects = 1;\n}\n// Request message for project operation.\nmessage QueryProjectRequest {\n string id = 1;\n}\n// Response message for project operation.\nmessage QueryProjectResponse {\n Project project = 1;\n}\n// Request message for projectStatuses operation.\nmessage QueryProjectStatusesRequest {\n}\n// Response message for projectStatuses operation.\nmessage QueryProjectStatusesResponse {\n repeated ProjectStatus project_statuses = 1;\n}\n// Request message for projectsByStatus operation.\nmessage QueryProjectsByStatusRequest {\n ProjectStatus status = 1;\n}\n// Response message for projectsByStatus operation.\nmessage QueryProjectsByStatusResponse {\n repeated Project projects_by_status = 1;\n}\n// Request message for projectResources operation.\nmessage QueryProjectResourcesRequest {\n string project_id = 1;\n}\n// Response message for projectResources operation.\nmessage QueryProjectResourcesResponse {\n repeated ProjectResource project_resources = 1;\n}\n// Request message for searchProjects operation.\nmessage QuerySearchProjectsRequest {\n string query = 1;\n}\n// Response message for searchProjects operation.\nmessage QuerySearchProjectsResponse {\n repeated ProjectSearchResult search_projects = 1;\n}\n// Request message for milestones operation.\nmessage QueryMilestonesRequest {\n string project_id = 1;\n}\n// Response message for milestones operation.\nmessage QueryMilestonesResponse {\n repeated Milestone milestones = 1;\n}\n// Request message for tasks operation.\nmessage QueryTasksRequest {\n string project_id = 1;\n}\n// Response message for tasks operation.\nmessage QueryTasksResponse {\n repeated Task tasks = 1;\n}\n// Request message for projectActivities operation.\nmessage QueryProjectActivitiesRequest {\n string project_id = 1;\n}\n// Response message for projectActivities operation.\nmessage QueryProjectActivitiesResponse {\n repeated ProjectActivity project_activities = 1;\n}\n// Request message for projectTags operation.\nmessage QueryProjectTagsRequest {\n}\n// Response message for projectTags operation.\nmessage QueryProjectTagsResponse {\n ListOfString project_tags = 1;\n}\n// Request message for archivedProjects operation.\nmessage QueryArchivedProjectsRequest {\n}\n// Response message for archivedProjects operation.\nmessage QueryArchivedProjectsResponse {\n repeated Project archived_projects = 1;\n}\n// Request message for tasksByPriority operation.\nmessage QueryTasksByPriorityRequest {\n string project_id = 1;\n}\n// Response message for tasksByPriority operation.\nmessage QueryTasksByPriorityResponse {\n ListOfListOfTask tasks_by_priority = 1;\n}\n// Request message for resourceMatrix operation.\nmessage QueryResourceMatrixRequest {\n string project_id = 1;\n}\n// Response message for resourceMatrix operation.\nmessage QueryResourceMatrixResponse {\n ListOfListOfProjectResource resource_matrix = 1;\n}\n// Request message for killService operation.\nmessage QueryKillServiceRequest {\n}\n// Response message for killService operation.\nmessage QueryKillServiceResponse {\n bool kill_service = 1;\n}\n// Request message for panic operation.\nmessage QueryPanicRequest {\n}\n// Response message for panic operation.\nmessage QueryPanicResponse {\n bool panic = 1;\n}\n// Request message for nodesById operation.\nmessage QueryNodesByIdRequest {\n string id = 1;\n}\n// Response message for nodesById operation.\nmessage QueryNodesByIdResponse {\n repeated Node nodes_by_id = 1;\n}\n// Request message for addProject operation.\nmessage MutationAddProjectRequest {\n ProjectInput project = 1;\n}\n// Response message for addProject operation.\nmessage MutationAddProjectResponse {\n Project add_project = 1;\n}\n// Request message for addMilestone operation.\nmessage MutationAddMilestoneRequest {\n MilestoneInput milestone = 1;\n}\n// Response message for addMilestone operation.\nmessage MutationAddMilestoneResponse {\n Milestone add_milestone = 1;\n}\n// Request message for addTask operation.\nmessage MutationAddTaskRequest {\n TaskInput task = 1;\n}\n// Response message for addTask operation.\nmessage MutationAddTaskResponse {\n Task add_task = 1;\n}\n// Request message for updateProjectStatus operation.\nmessage MutationUpdateProjectStatusRequest {\n string project_id = 1;\n ProjectStatus status = 2;\n}\n// Response message for updateProjectStatus operation.\nmessage MutationUpdateProjectStatusResponse {\n ProjectUpdate update_project_status = 1;\n}\n\nmessage Project {\n string id = 1 [(is_required) = true];\n string name = 2 [(is_required) = true];\n google.protobuf.StringValue description = 3;\n google.protobuf.StringValue start_date = 4;\n google.protobuf.StringValue end_date = 5;\n ProjectStatus status = 6 [(is_required) = true];\n repeated Employee team_members = 7 [(is_required) = true];\n repeated Product related_products = 8 [(is_required) = true];\n ListOfString milestone_ids = 9;\n repeated Milestone milestones = 10 [(is_required) = true];\n repeated Task tasks = 11 [(is_required) = true];\n google.protobuf.DoubleValue progress = 12;\n ListOfString tags = 13;\n ListOfProject alternative_projects = 14;\n ListOfProject dependencies = 15;\n ListOfListOfProjectResource resource_groups = 16 [(is_required) = true];\n ListOfListOfTask tasks_by_phase = 17 [(is_required) = true];\n ListOfListOfMilestone milestone_groups = 18;\n ListOfListOfListOfTask priority_matrix = 19;\n}\n\nmessage Milestone {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n string name = 3 [(is_required) = true];\n google.protobuf.StringValue description = 4;\n google.protobuf.StringValue start_date = 5;\n google.protobuf.StringValue end_date = 6;\n MilestoneStatus status = 7 [(is_required) = true];\n google.protobuf.DoubleValue completion_percentage = 8;\n repeated Milestone dependencies = 9 [(is_required) = true];\n ListOfTask subtasks = 10;\n ListOfEmployee reviewers = 11;\n}\n\nmessage Task {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n google.protobuf.StringValue milestone_id = 3;\n google.protobuf.Int32Value assignee_id = 4;\n string name = 5 [(is_required) = true];\n google.protobuf.StringValue description = 6;\n TaskPriority priority = 7 [(is_required) = true];\n TaskStatus status = 8 [(is_required) = true];\n // Deprecation notice: No more estimations!\n google.protobuf.DoubleValue estimated_hours = 9 [deprecated = true];\n google.protobuf.DoubleValue actual_hours = 10;\n google.protobuf.StringValue created_at = 11;\n google.protobuf.StringValue completed_at = 12;\n ListOfString labels = 13;\n ListOfTask subtasks = 14;\n repeated Task dependencies = 15 [(is_required) = true];\n repeated string attachment_urls = 16 [(is_required) = true];\n ListOfInt reviewer_ids = 17;\n}\n\nmessage Employee {\n int32 id = 1 [(is_required) = true];\n ListOfProject projects = 2;\n repeated Task assigned_tasks = 3 [(is_required) = true];\n repeated Task completed_tasks = 4 [(is_required) = true];\n ListOfString skills = 5;\n ListOfString certifications = 6;\n ListOfListOfProject project_history = 7 [(is_required) = true];\n}\n\nmessage Product {\n string upc = 1 [(is_required) = true];\n ListOfProject projects = 2;\n ListOfListOfString feature_matrix = 3;\n}\n\nenum ProjectStatus {\n PROJECT_STATUS_UNSPECIFIED = 0;\n PROJECT_STATUS_PLANNING = 1;\n PROJECT_STATUS_ACTIVE = 2;\n PROJECT_STATUS_COMPLETED = 3;\n PROJECT_STATUS_ON_HOLD = 4;\n}\n\nmessage ProjectResource {\n oneof value {\n Employee employee = 1;\n Product product = 2;\n Milestone milestone = 3;\n Task task = 4;\n }\n}\n\nmessage ProjectSearchResult {\n oneof value {\n Project project = 1;\n Milestone milestone = 2;\n Task task = 3;\n }\n}\n\nmessage ProjectActivity {\n oneof value {\n ProjectUpdate project_update = 1;\n Milestone milestone = 2;\n Task task = 3;\n }\n}\n\nmessage Node {\n oneof instance {\n Project project = 1;\n Milestone milestone = 2;\n Task task = 3;\n ProjectUpdate project_update = 4;\n }\n}\n\nmessage ProjectInput {\n string name = 1 [(is_required) = true];\n google.protobuf.StringValue description = 2;\n google.protobuf.StringValue start_date = 3;\n google.protobuf.StringValue end_date = 4;\n ProjectStatus status = 5 [(is_required) = true];\n}\n\nmessage MilestoneInput {\n string project_id = 1 [(is_required) = true];\n string name = 2 [(is_required) = true];\n google.protobuf.StringValue description = 3;\n google.protobuf.StringValue due_date = 4;\n MilestoneStatus status = 5 [(is_required) = true];\n}\n\nmessage TaskInput {\n string project_id = 1 [(is_required) = true];\n google.protobuf.Int32Value assignee_id = 2;\n string name = 3 [(is_required) = true];\n google.protobuf.StringValue description = 4;\n TaskPriority priority = 5 [(is_required) = true];\n TaskStatus status = 6 [(is_required) = true];\n google.protobuf.DoubleValue estimated_hours = 7;\n}\n\nmessage ProjectUpdate {\n string id = 1 [(is_required) = true];\n string project_id = 2 [(is_required) = true];\n int32 updated_by_id = 3 [(is_required) = true];\n ProjectUpdateType update_type = 4 [(is_required) = true];\n string description = 5 [(is_required) = true];\n string timestamp = 6 [(is_required) = true];\n google.protobuf.StringValue metadata = 7;\n}\n\nmessage Timestamped {\n oneof instance {\n Project project = 1;\n Milestone milestone = 2;\n }\n}\n\nmessage Assignable {\n oneof instance {\n Task task = 1;\n }\n}\n\nenum MilestoneStatus {\n MILESTONE_STATUS_UNSPECIFIED = 0;\n MILESTONE_STATUS_PENDING = 1;\n MILESTONE_STATUS_IN_PROGRESS = 2;\n MILESTONE_STATUS_COMPLETED = 3;\n MILESTONE_STATUS_DELAYED = 4;\n}\n\nenum TaskStatus {\n TASK_STATUS_UNSPECIFIED = 0;\n TASK_STATUS_TODO = 1;\n TASK_STATUS_IN_PROGRESS = 2;\n TASK_STATUS_REVIEW = 3;\n TASK_STATUS_COMPLETED = 4;\n TASK_STATUS_BLOCKED = 5;\n}\n\nenum TaskPriority {\n TASK_PRIORITY_UNSPECIFIED = 0;\n TASK_PRIORITY_LOW = 1;\n TASK_PRIORITY_MEDIUM = 2;\n TASK_PRIORITY_HIGH = 3;\n TASK_PRIORITY_URGENT = 4;\n}\n\nenum ProjectUpdateType {\n PROJECT_UPDATE_TYPE_UNSPECIFIED = 0;\n PROJECT_UPDATE_TYPE_STATUS_CHANGE = 1;\n PROJECT_UPDATE_TYPE_MILESTONE_ADDED = 2;\n PROJECT_UPDATE_TYPE_TASK_ASSIGNED = 3;\n PROJECT_UPDATE_TYPE_PROGRESS_UPDATE = 4;\n PROJECT_UPDATE_TYPE_TEAM_CHANGE = 5;\n}"
}
import "google/protobuf/descriptor.proto";
import "google/protobuf/wrappers.proto";
extend google.protobuf.FieldOptions {
bool is_required = 50000;
}
// Service definition for ProjectsService
service ProjectsService {
// …
}
🤖 Prompt for AI Agents
In router-tests/testenv/testdata/configWithGRPC.json around lines 1796-1797,
inside the protoSchema string locate the extend google.protobuf.FieldOptions
block and remove the "optional" label from the custom option declaration so it
reads as a plain field declaration (bool is_required = 50000;). Update the
protoSchema string accordingly (preserve surrounding quoting/escaping) and save
the file.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
router-tests/testenv/testdata/configWithGRPC.json (1)

1796-1796: Drop optional from the FieldOptions extension and prefer fully-qualified option refs (recommended)

Short: I ran the repo search you requested — the custom FieldOptions extension is_required is used widely and currently declared with optional bool is_required = 50000;. To reduce proto3/tooling compatibility risk, update the extension declaration in the generator and regenerate artifacts; optionally qualify usages as (service.is_required) to avoid name clashes.

What I found (verification):

  • The extension declaration/current usage occurs in:
    • protographic/src/sdl-to-proto-visitor.ts (generator adds the extension) — update here so generated protos stop including optional.
    • demo/pkg/subgraphs/projects/generated/service.proto (generated proto contains the extension).
    • demo/pkg/subgraphs/projects/generated/service.pb.go (generated Go uses E_IsRequired).
    • router-tests/testenv/testdata/configWithGRPC.json and router-tests/testenv/testdata/configWithPlugins.json (embedded protoSchema strings).
    • Many test fixtures: protographic/tests/sdl-to-proto/*.test.ts (multiple occurrences of the same extension and of [(is_required) = true] on fields).
  • No other differing extension names or different option numbers were discovered — all occurrences reference the same is_required = 50000 extension (i.e., no conflicting option numbers found).

Recommended minimal change (update generator + fixtures / regen):

-extend google.protobuf.FieldOptions {
-  optional bool is_required = 50000;
-}
+extend google.protobuf.FieldOptions {
+  bool is_required = 50000;
+}

Optional hardening (qualify option usages across protos / fixtures):

- string id = 1 [(is_required) = true];
+ string id = 1 [(service.is_required) = true];

Action items:

  • Update protographic/src/sdl-to-proto-visitor.ts to emit the non-optional extension and (optionally) qualified option references.
  • Regenerate demo/pkg/subgraphs/projects/generated/service.proto and the Go code (service.pb.go).
  • Update test fixtures under protographic/tests/sdl-to-proto/* and the testdata JSON files (or update tests) to match the new output.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 36d68e8 and 9b00ee9.

⛔ Files ignored due to path filters (2)
  • demo/pkg/subgraphs/projects/generated/service.pb.go is excluded by !**/*.pb.go, !**/generated/**
  • demo/pkg/subgraphs/projects/generated/service.proto is excluded by !**/generated/**
📒 Files selected for processing (1)
  • router-tests/testenv/testdata/configWithGRPC.json (2 hunks)
🔇 Additional comments (2)
router-tests/testenv/testdata/configWithGRPC.json (2)

2202-2202: Version bump LGTM

Version updated to bead767c-5717-462b-9bf5-dbe3ea94aa84. No concerns from the router/testenv perspective.


1796-1796: Ensure the router’s proto loader pre-loads well-known types

I didn’t find any includePaths or similar settings in the router code or test harness that would allow descriptor.proto and wrappers.proto to resolve at load time. By contrast, our Protographic TS tests explicitly do:

  • protographic/tests/util.ts
    root.loadSync(path.join(__dirname, 'google/protobuf/descriptor.proto'));
    root.loadSync('google/protobuf/wrappers.proto');

Without a matching setup in the router harness, loading the protoSchema from configWithGRPC.json (which imports both WKT files) may fail with “file not found” errors.

Please verify that the router’s loader is configured to:

  • Pass the well-known include paths (e.g. via protoparse’s ImportPaths or grpc-tools’ include settings), or
  • Pre-load google/protobuf/descriptor.proto and google/protobuf/wrappers.proto into the parsing root

You can confirm this by running the router binary or test harness against router-tests/testenv/testdata/configWithGRPC.json and ensuring no import resolution errors occur.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
router-tests/testenv/testdata/configWithGRPC.json (1)

1796-1796: Add (is_required) to Project.priority_matrix to match GraphQL non-null outer list.

The GraphQL SDL defines priorityMatrix: [[[Task!]!]!] (outer list non-null), but the proto field lacks (is_required) = true. This makes the proto inconsistent with intended non-nullability and with the rest of the annotated fields.

Apply this diff inside the protoSchema string:

-  ListOfListOfListOfTask priority_matrix = 19;
+  ListOfListOfListOfTask priority_matrix = 19 [(is_required) = true];
🧹 Nitpick comments (5)
router-tests/testenv/testdata/configWithGRPC.json (1)

1796-1796: Consider centralizing the is_required extension to avoid duplicate symbol conflicts across multiple protos.

Emitting the extend google.protobuf.FieldOptions { bool is_required = 50000; } block in every generated proto file risks duplicate extension symbol definitions when multiple generated files are compiled into the same binary. Prefer a single shared annotations.proto (or similar) that defines the extension and have generated files import it. Alternatively, guard header emission behind a configuration that allows externalizing the extension file.

protographic/tests/sdl-to-proto/12-directive.test.ts (3)

33-38: Stabilize option ordering to reduce snapshot churn.

Field option ordering alternates between [deprecated = true, (is_required) = true] and [(is_required) = true, deprecated = true] across tests. Consider enforcing a deterministic order (e.g., deprecated first, then custom options) in the formatter to avoid unnecessary diffs.

Example target order:

  • Object types: [deprecated = true, (is_required) = true]
  • Input types: same ordering for consistency

Also applies to: 331-336


192-196: Fix typo in test title (“should should”).

Minor nit: duplicate word in the test name.

-  it('should should ignore empty reason on field when interface has a reason', () => {
+  it('should ignore empty reason on field when interface has a reason', () => {

1-604: Add a negative case: when includeRequiredAnnotations is disabled, no header or annotations should be emitted.

To fully exercise the new option, add a test asserting the absence of descriptor.proto import, extension block, and (is_required) when includeRequiredAnnotations: false.

Here’s a test you can append to this file:

it('should omit required annotations when disabled', () => {
  const sdl = `
    type User {
      id: ID!
      name: String!
      email: String
    }
  `;
  const { proto: protoText } = compileGraphQLToProto(sdl, {
    includeRequiredAnnotations: false,
  });

  expectValidProto(protoText);

  // No descriptor import and no extension block
  expect(protoText).not.toContain('google/protobuf/descriptor.proto');
  expect(protoText).not.toContain('extend google.protobuf.FieldOptions');

  // No (is_required) options on fields
  expect(protoText).not.toMatch(/\(is_required\)\s*=\s*true/);
});
protographic/tests/sdl-to-proto/09-comments.test.ts (1)

85-91: Ensure no duplicate is_required extension across protos

  • We located inline extend google.protobuf.FieldOptions { bool is_required = 50000; } in:
    • demo/pkg/subgraphs/projects/generated/service.proto
    • router-tests/testenv/testdata/configWithGRPC.json
    • router-tests/testenv/testdata/configWithPlugins.json
    • protographic/tests/sdl-to-proto/**/*.test.ts (test fixtures)
  • All use field number 50000, so numbering is consistent.
  • Risk: If multiple generated or hand-written .proto files in the same package declare the unqualified is_required extension, protoc will error on duplicate symbol definitions.
  • Recommendation: Extract the extension into a single shared options.proto (e.g.
    syntax = "proto3";
    package wundergraph.options;
    import "google/protobuf/descriptor.proto";
    extend google.protobuf.FieldOptions {
      bool is_required = 50000;
    }
    ) and have your generator import that file in each output.
    Or namespace the option—e.g. option (wundergraph.is_required) = true;—to avoid global collisions.
  • Minor: Only emit the import "google/protobuf/descriptor.proto"; and extend … block when includeRequiredAnnotations is enabled and at least one field is annotated, to prevent dead imports.
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9b00ee9 and ccc7a65.

⛔ Files ignored due to path filters (2)
  • demo/pkg/subgraphs/projects/generated/service.pb.go is excluded by !**/*.pb.go, !**/generated/**
  • demo/pkg/subgraphs/projects/generated/service.proto is excluded by !**/generated/**
📒 Files selected for processing (11)
  • protographic/src/sdl-to-proto-visitor.ts (12 hunks)
  • protographic/tests/sdl-to-proto/01-basic-types.test.ts (10 hunks)
  • protographic/tests/sdl-to-proto/02-complex-types.test.ts (10 hunks)
  • protographic/tests/sdl-to-proto/03-interfaces-unions.test.ts (8 hunks)
  • protographic/tests/sdl-to-proto/04-federation.test.ts (16 hunks)
  • protographic/tests/sdl-to-proto/05-edge-cases.test.ts (7 hunks)
  • protographic/tests/sdl-to-proto/09-comments.test.ts (8 hunks)
  • protographic/tests/sdl-to-proto/10-options.test.ts (1 hunks)
  • protographic/tests/sdl-to-proto/11-lists.test.ts (37 hunks)
  • protographic/tests/sdl-to-proto/12-directive.test.ts (16 hunks)
  • router-tests/testenv/testdata/configWithGRPC.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • protographic/tests/sdl-to-proto/05-edge-cases.test.ts
  • protographic/tests/sdl-to-proto/10-options.test.ts
  • protographic/tests/sdl-to-proto/01-basic-types.test.ts
  • protographic/tests/sdl-to-proto/11-lists.test.ts
  • protographic/src/sdl-to-proto-visitor.ts
  • protographic/tests/sdl-to-proto/02-complex-types.test.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T10:38:49.165Z
Learnt from: Noroth
PR: wundergraph/cosmo#2151
File: protographic/src/sdl-to-proto-visitor.ts:0-0
Timestamp: 2025-08-20T10:38:49.165Z
Learning: Inline `extend google.protobuf.FieldOptions` declarations in proto3 files work correctly with protoc, despite theoretical concerns about proto3 compatibility with extension declarations.

Applied to files:

  • protographic/tests/sdl-to-proto/12-directive.test.ts
  • protographic/tests/sdl-to-proto/03-interfaces-unions.test.ts
  • protographic/tests/sdl-to-proto/09-comments.test.ts
🧬 Code Graph Analysis (1)
protographic/tests/sdl-to-proto/12-directive.test.ts (2)
protographic/src/index.ts (1)
  • compileGraphQLToProto (53-79)
protographic/tests/util.ts (1)
  • expectValidProto (36-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: build_push_image
  • GitHub Check: integration_test (./events)
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: image_scan
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: build_test
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
router-tests/testenv/testdata/configWithGRPC.json (2)

1796-1796: Proto3 custom option declaration is correct (no “optional” keyword).

The header correctly imports google/protobuf/descriptor.proto and declares extend google.protobuf.FieldOptions { bool is_required = 50000; } without the proto2-style optional label. This is the right shape for proto3 custom options and should parse across protoc versions.


2202-2202: Version bump acknowledged.

The config version was updated; no issues.

protographic/tests/sdl-to-proto/04-federation.test.ts (4)

41-46: LGTM: Descriptor import and is_required extension header added.

The snapshots correctly include google/protobuf/descriptor.proto and the is_required FieldOptions extension. This aligns with the new required-field annotations.


134-143: Required annotations applied consistently to entity fields.

Product.id/name/price and User.id/name/email now carry (is_required) = true, matching GraphQL non-nullability. Good coverage.


422-428: Wrappers imported where needed for special scalars — correct.

The snapshots import wrappers.proto and use wrapper types for optional scalars. Required fields have (is_required) = true. Looks good.

Also applies to: 451-458


31-33: includeRequiredAnnotations defaults to true by design

The GraphQLToProtoTextVisitor constructor destructures its options with includeRequiredAnnotations = true, and all calls to compileGraphQLToProto pass through that same options object. Tests that rely on (is_required) without explicitly setting includeRequiredAnnotations are consistent with the intended default behavior, so no changes are necessary.

protographic/tests/sdl-to-proto/03-interfaces-unions.test.ts (2)

40-45: LGTM: Proto header includes descriptor import and is_required extension.

The snapshot headers are correct and consistent with the feature.


75-85: Entity fields annotated as required — matches GraphQL non-nullability.

User.id/name/email and Product.id/name/price carry (is_required) = true. Consistent and expected.

Also applies to: 81-85

protographic/tests/sdl-to-proto/12-directive.test.ts (2)

23-28: LGTM: Added descriptor import and is_required extension where required.

Header looks correct across directive/deprecation scenarios.


565-602: Great addition: validates coexistence of deprecated and required annotations.

This adds valuable coverage ensuring both options coexist on the same field, including wrapper usage for optional fields.

protographic/tests/sdl-to-proto/09-comments.test.ts (1)

144-144: Required annotation usage looks correct and consistent with GraphQL non-null semantics

  • Correct: is_required is set only on fields derived from non-null types (e.g., id: ID!, name/email/name/price: non-null scalars).
  • Correct: Optional scalars continue to use wrappers (StringValue, Int32Value) without is_required.
  • Correct: Non-null scalars use native proto types without wrappers and carry is_required.

Two follow-ups to consider:

  • Should GraphQL arguments that are non-null (e.g., QueryUserRequest.id, QuerySearchRequest.query) also receive is_required? If the intent is “per-field option for all non-null GraphQL-origin fields,” arguments currently remain unannotated. If intentionally excluded, let’s document that scope.
  • If you decide to include arguments, ensure the generator uniformly applies the option to Request message fields as well to avoid drift between object/input fields and argument fields.

Also applies to: 171-173, 312-314, 320-322, 507-511, 520-524

@Noroth Noroth closed this Aug 25, 2025
@Noroth Noroth deleted the ludwig/eng-7490-protographic-add-annotation-to-required-fields branch August 25, 2025 08:44
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