feat(cli,protographic): improvements to operation validation and proto generation

cli/src/commands/grpc-service/commands/generate.ts

@@ -27,7 +27,6 @@ type CLIOptions = {
   customScalarMapping?: string;
   customScalarMappingFile?: string;
   maxDepth?: string;
-  prefixOperationType?: boolean;
 } & ProtoOptions;
 
 export default (opts: BaseCommandOptions) => {
@@ -72,11 +71,6 @@ export default (opts: BaseCommandOptions) => {
     'Maximum recursion depth for processing nested selections and fragments (default: 50). ' +
       'Increase this if you have deeply nested queries or decrease to catch potential circular references earlier.',
   );
-  command.option(
-    '--prefix-operation-type',
-    'Prefix RPC method names with the operation type (Query/Mutation). Only applies with --with-operations. ' +
-      'Subscriptions are not prefixed.',
-  );
   command.action(generateCommandAction);
 
   return command;
@@ -163,11 +157,6 @@ async function generateCommandAction(name: string, options: CLIOptions) {
       maxDepth = parsed;
     }
 
-    // Validate prefix-operation-type usage
-    if (options.prefixOperationType && !options.withOperations) {
-      spinner.warn('--prefix-operation-type flag is ignored when not using --with-operations');
-    }
-
     const languageOptions: ProtoOptions = {
       goPackage: options.goPackage,
       javaPackage: options.javaPackage,
@@ -192,7 +181,6 @@ async function generateCommandAction(name: string, options: CLIOptions) {
       operationsDir: options.withOperations,
       customScalarMappings,
       maxDepth,
-      prefixOperationType: options.prefixOperationType,
     });
 
     // Write the generated files
@@ -244,7 +232,6 @@ type GenerationOptions = {
   operationsDir?: string;
   customScalarMappings?: Record<string, string>;
   maxDepth?: number;
-  prefixOperationType?: boolean;
 };
 
 /**
@@ -335,7 +322,6 @@ function mergeProtoRoots(roots: protobuf.Root[], serviceName: string): protobuf.
  * @param lockFile - Path to the proto lock file
  * @param customScalarMappings - Custom scalar type mappings
  * @param maxDepth - Maximum recursion depth
- * @param prefixOperationType - Whether to prefix operation types
  * @returns Generation result with proto content and lock data
  * @note All Query operations are automatically marked with NO_SIDE_EFFECTS idempotency level
  */
@@ -349,7 +335,6 @@ async function generateFromOperations(
   lockFile: string,
   customScalarMappings?: Record<string, string>,
   maxDepth?: number,
-  prefixOperationType?: boolean,
 ): Promise<GenerationResult> {
   spinner.text = 'Reading operation files...';
   const operationFiles = await readOperationFiles(operationsPath);
@@ -359,8 +344,8 @@ async function generateFromOperations(
   // Load lock data for field number stability
   let currentLockData = await fetchLockData(lockFile);
 
-  // Process each operation file separately to maintain reversibility
-  // Collect the AST roots instead of proto strings
+  // Process each operation file separately, updating lock data sequentially for field number stability.
+  // Collect AST roots for merging rather than proto strings.
   const roots: protobuf.Root[] = [];
 
   for (const { filename, content } of operationFiles) {
@@ -376,7 +361,6 @@ async function generateFromOperations(
         lockData: currentLockData,
         customScalarMappings,
         maxDepth,
-        prefixOperationType,
       });
 
       // Keep the AST root instead of the string
@@ -495,7 +479,6 @@ async function generateProtoAndMapping({
   operationsDir,
   customScalarMappings,
   maxDepth,
-  prefixOperationType,
 }: GenerationOptions): Promise<GenerationResult> {
   const schema = await readFile(schemaFile, 'utf8');
   const serviceName = upperFirst(camelCase(name));
@@ -518,7 +501,6 @@ async function generateProtoAndMapping({
       lockFile,
       customScalarMappings,
       maxDepth,
-      prefixOperationType,
     );
   } else {
     return generateFromSDL(schema, serviceName, spinner, packageName, languageOptions, lockFile);

protographic/OPERATIONS_TO_PROTO.md

@@ -716,6 +716,6 @@ Field number conflict in message X
 ## Limitations
 
 - **Named operations only**: All operations must have a name. Anonymous operations are not supported
-- **Single operation per document**: Multiple operations in one document are not supported for proto reversibility
-- **No root-level field aliases**: Field aliases at the root level break proto-to-GraphQL reversibility
+- **Single operation per document**: Multiple operations in one document are not supported for deterministic proto schema generation
+- **No root-level field aliases**: Field aliases at the root level break proto schema generation consistency (each GraphQL field must map to exactly one proto field name)
 - **Alpha status**: The API may change in future releases

protographic/src/naming-conventions.ts

@@ -26,6 +26,14 @@ export function graphqlArgumentToProtoField(argName: string): string {
   return snakeCase(argName);
 }
 
+/**
+ * Converts a proto field name (snake_case) to the expected protobuf JSON format (camelCase)
+ * This matches how protobuf JSON marshaling automatically converts field names
+ */
+export function protoFieldToProtoJSON(protoFieldName: string): string {
+  return camelCase(protoFieldName);
+}
+
 /**
  * Creates an operation method name from an operation type and field name
  */

protographic/src/operation-to-proto.ts

@@ -104,7 +104,7 @@ export function compileOperationsToProto(
     const operationNames = namedOperations.map((op) => (op as OperationDefinitionNode).name!.value).join(', ');
     throw new Error(
       `Multiple operations found in document: ${operationNames}. ` +
-        'Only a single named operation per document is supported for proto reversibility. ' +
+        'Only a single named operation per document is supported for deterministic proto schema generation. ' +
         'Please compile each operation separately.',
     );
   }
@@ -264,21 +264,37 @@ class OperationsToProtoVisitor {
       return;
     }
 
-    // 2. Validate no root-level field aliases (breaks reversibility)
+    // 2. Validate operation name starts with uppercase letter
+    // This follows protobuf RPC naming conventions while allowing flexibility
+    // Must start with uppercase letter, followed by letters/numbers
+    if (!/^[A-Z][a-zA-Z0-9]*$/.test(operationName)) {
+      const suggestedName = upperFirst(camelCase(operationName));
+      throw new Error(
+        `Operation name "${operationName}" must start with an uppercase letter ` +
+          `and contain only letters and numbers. ` +
+          `Examples: GetUser, CreatePost, HRService, GETUSER. ` +
+          `Suggested name: "${suggestedName}". ` +
+          `This follows protobuf RPC naming conventions.`,
+      );
+    }
+
+    // 3. Validate no root-level field aliases (breaks proto schema consistency)
     if (node.selectionSet) {
       for (const selection of node.selectionSet.selections) {
         if (selection.kind === 'Field' && selection.alias) {
           throw new Error(
             `Root-level field alias "${selection.alias.value}: ${selection.name.value}" is not supported. ` +
-              'Field aliases at the root level break proto-to-GraphQL reversibility. ' +
+              'Field aliases at the root level break proto schema generation consistency. ' +
+              'Each GraphQL field must map to exactly one proto field name. ' +
               'Please remove the alias or use it only on nested fields.',
           );
         }
       }
     }
 
-    // 3. Create method name from operation name, optionally prefixed with operation type
-    let methodName = upperFirst(camelCase(operationName));
+    // 4. Create method name from operation name
+    // Use operation name as-is to ensure exact matching (no transformation)
+    let methodName = operationName;
 
     // Add operation type prefix if requested
     if (this.prefixOperationType) {

protographic/src/operations/message-builder.ts

@@ -78,10 +78,15 @@ export function buildMessageFromSelectionSet(
   parentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType,
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
+  parentPath?: string,
 ): protobuf.Type {
   const message = new protobuf.Type(messageName);
   const fieldNumberManager = options?.fieldNumberManager;
 
+  // Build the full path for lock file tracking (e.g., "ParentMessage.ChildMessage")
+  // This is used for field number management to prevent collisions between nested messages
+  const fullPath = parentPath ? `${parentPath}.${messageName}` : messageName;
+
   // First pass: collect all field names that will be in this message
   const fieldNames: string[] = [];
   const fieldSelections = new Map<string, { selection: FieldNode; type: GraphQLObjectType | GraphQLInterfaceType }>();
@@ -168,14 +173,15 @@ export function buildMessageFromSelectionSet(
   collectFields(selectionSet.selections, parentType, currentDepth);
 
   // Reconcile field order using lock manager if available
+  // Use fullPath for lock file operations to track nested message hierarchy
   let orderedFieldNames = fieldNames;
   if (fieldNumberManager && 'reconcileFieldOrder' in fieldNumberManager) {
-    orderedFieldNames = fieldNumberManager.reconcileFieldOrder(messageName, fieldNames);
+    orderedFieldNames = fieldNumberManager.reconcileFieldOrder(fullPath, fieldNames);
   }
 
   // Second pass: process fields in reconciled order
   // Pre-assign field numbers from lock data if available
-  assignFieldNumbersFromLockData(messageName, orderedFieldNames, fieldNumberManager);
+  assignFieldNumbersFromLockData(fullPath, orderedFieldNames, fieldNumberManager);
 
   for (const protoFieldName of orderedFieldNames) {
     const fieldData = fieldSelections.get(protoFieldName);
@@ -184,7 +190,15 @@ export function buildMessageFromSelectionSet(
         ...options,
         _depth: currentDepth,
       };
-      processFieldSelection(fieldData.selection, message, fieldData.type, typeInfo, fieldOptions, fieldNumberManager);
+      processFieldSelection(
+        fieldData.selection,
+        message,
+        fieldData.type,
+        typeInfo,
+        fieldOptions,
+        fieldNumberManager,
+        fullPath,
+      );
     }
   }
 
@@ -195,23 +209,24 @@ export function buildMessageFromSelectionSet(
  * Gets or assigns a field number for a proto field
  */
 function getOrAssignFieldNumber(
-  message: protobuf.Type,
+  messagePath: string,
   protoFieldName: string,
   fieldNumberManager?: FieldNumberManager,
+  message?: protobuf.Type,
 ): number {
-  const existingFieldNumber = fieldNumberManager?.getFieldNumber(message.name, protoFieldName);
+  const existingFieldNumber = fieldNumberManager?.getFieldNumber(messagePath, protoFieldName);
 
   if (existingFieldNumber !== undefined) {
     return existingFieldNumber;
   }
 
   if (fieldNumberManager) {
-    const fieldNumber = fieldNumberManager.getNextFieldNumber(message.name);
-    fieldNumberManager.assignFieldNumber(message.name, protoFieldName, fieldNumber);
+    const fieldNumber = fieldNumberManager.getNextFieldNumber(messagePath);
+    fieldNumberManager.assignFieldNumber(messagePath, protoFieldName, fieldNumber);
     return fieldNumber;
   }
 
-  return message.fieldsArray.length + 1;
+  return message ? message.fieldsArray.length + 1 : 1;
 }
 
 /**
@@ -292,6 +307,7 @@ function processFieldSelection(
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
   fieldNumberManager?: FieldNumberManager,
+  messagePath?: string,
 ): void {
   const fieldName = field.name.value;
 
@@ -332,19 +348,25 @@ function processFieldSelection(
     // Build nested message for object types
     const namedType = getNamedType(fieldType);
     if (isObjectType(namedType) || isInterfaceType(namedType) || isUnionType(namedType)) {
+      // Use clean nested message name (just the field name in PascalCase)
+      // The full path will be tracked separately for lock file operations
       const nestedMessageName = upperFirst(camelCase(fieldName));
 
       const nestedOptions = {
         ...options,
         _depth: (options?._depth ?? 0) + 1,
       };
 
+      // Pass the current message path to build the full hierarchy for lock file tracking
+      const currentPath = messagePath || message.name;
+
       const nestedMessage = buildMessageFromSelectionSet(
         nestedMessageName,
         field.selectionSet,
         namedType,
         typeInfo,
         nestedOptions,
+        currentPath, // Pass parent path for nested message tracking
       );
 
       message.add(nestedMessage);
@@ -373,7 +395,9 @@ function processFieldSelection(
 
   const { typeName, isRepeated } = resolveTypeNameAndRepetition(baseTypeName, protoTypeInfo, fieldType, options);
 
-  const fieldNumber = getOrAssignFieldNumber(message, protoFieldName, fieldNumberManager);
+  // Use the full message path for field number assignment to prevent collisions
+  const currentPath = messagePath || message.name;
+  const fieldNumber = getOrAssignFieldNumber(currentPath, protoFieldName, fieldNumberManager, message);
 
   const protoField = createProtoField(protoFieldName, fieldNumber, typeName, isRepeated, fieldDef, options);
 
@@ -391,6 +415,7 @@ function processInlineFragment(
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
   fieldNumberManager?: FieldNumberManager,
+  messagePath?: string,
 ): void {
   // Determine the type for this inline fragment
   let fragmentType: GraphQLObjectType | GraphQLInterfaceType | GraphQLUnionType;
@@ -421,12 +446,12 @@ function processInlineFragment(
   if (fragment.selectionSet) {
     for (const selection of fragment.selectionSet.selections) {
       if (selection.kind === 'Field') {
-        processFieldSelection(selection, message, fragmentType, typeInfo, options, fieldNumberManager);
+        processFieldSelection(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath);
       } else if (selection.kind === 'InlineFragment') {
         // Nested inline fragment
-        processInlineFragment(selection, message, fragmentType, typeInfo, options, fieldNumberManager);
+        processInlineFragment(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath);
       } else if (selection.kind === 'FragmentSpread') {
-        processFragmentSpread(selection, message, fragmentType, typeInfo, options, fieldNumberManager);
+        processFragmentSpread(selection, message, fragmentType, typeInfo, options, fieldNumberManager, messagePath);
       }
     }
   }
@@ -443,6 +468,7 @@ function processFragmentSpread(
   typeInfo: TypeInfo,
   options?: MessageBuilderOptions,
   fieldNumberManager?: FieldNumberManager,
+  messagePath?: string,
 ): void {
   const fragmentName = spread.name.value;
   const fragments = options?.fragments;
@@ -476,12 +502,12 @@ function processFragmentSpread(
   // Process the fragment's selection set with the resolved type
   for (const selection of fragmentDef.selectionSet.selections) {
     if (selection.kind === 'Field') {
-      processFieldSelection(selection, message, type, typeInfo, options, fieldNumberManager);
+      processFieldSelection(selection, message, type, typeInfo, options, fieldNumberManager, messagePath);
     } else if (selection.kind === 'InlineFragment') {
-      processInlineFragment(selection, message, type, typeInfo, options, fieldNumberManager);
+      processInlineFragment(selection, message, type, typeInfo, options, fieldNumberManager, messagePath);
     } else if (selection.kind === 'FragmentSpread') {
       // Nested fragment spread (fragment inside fragment)
-      processFragmentSpread(selection, message, type, typeInfo, options, fieldNumberManager);
+      processFragmentSpread(selection, message, type, typeInfo, options, fieldNumberManager, messagePath);
     }
   }
 }

protographic/src/operations/proto-field-options.ts

@@ -0,0 +1,19 @@
+/**
+ * Protocol Buffer field option constants for ConnectRPC integration.
+ *
+ * These options are defined using `extend google.protobuf.FieldOptions` in generated proto files.
+ */
+
+export interface ProtoFieldOption {
+  readonly fieldNumber: number;
+  readonly optionName: string;
+}
+
+/**
+ * Maps protobuf fields to GraphQL variable names when they don't match the expected format.
+ * Field number 50001 is in the user-defined extension range.
+ */
+export const GRAPHQL_VARIABLE_NAME: ProtoFieldOption = {
+  fieldNumber: 50001,
+  optionName: '(graphql_variable_name)',
+} as const;