Pin protobuf field and enum value numbers in a sidecar artifact by jwils · Pull Request #1304 · block/elasticgraph

jwils · 2026-07-08T23:21:43Z

Why

With #1080's sequential numbering, reordering or removing a field renumbers everything after it — breaking wire compatibility with previously serialized data. Protobuf field numbers must stay stable across schema evolution.

What

schema_artifacts:dump now reads and writes a proto_field_numbers.yaml sidecar artifact: existing numbers stay fixed even if field order changes, and new fields allocate from each message's stored next_number without backfilling earlier gaps
schema.proto and the sidecar both use public GraphQL field names; name_in_index remains an indexing-time concern
a field renamed with field.renamed_from reuses its existing number under the new public name
enum value numbers are pinned in an enums section; removed values keep their numbers reserved so they are never reused (0 remains the generated *_UNSPECIFIED value)
mappings are validated: collisions raise clear errors, and non-integer numbers are rejected instead of letting Integer() silently truncate

Risk Assessment

Low — only affects the unreleased elasticgraph-proto_ingestion extension from #1080.

References

Stacked on Generate a protobuf schema artifact from ElasticGraph schemas #1080

Update — 2026-07-15

Merged the latest Generate a protobuf schema artifact from ElasticGraph schemas #1080 generator refactor so reachable types are selected before rendering.
Interface and union oneof alternatives now use the same stable message-field map: existing and removed subtype tags remain reserved, while new subtypes receive the next available tag.
Verification: all 68 elasticgraph-proto_ingestion examples pass with 100% line and branch coverage; GraphQL-schema validation, lint, spelling, and Steep are green.

Update — 2026-07-29

Restacked on Provide path_to_schema to schema definition state #1316, which exposes path_to_schema through schema definition state without expanding SchemaArtifactManager's dependencies.
Existing number mappings now load when the proto state extension initializes, before results can be generated or memoized.
Verification: all 71 elasticgraph-proto_ingestion examples pass with 100% line and branch coverage; lint and Steep are green.

Update — 2026-08-07

Kept the field-number sidecar focused on protobuf wire state: every message field is represented uniformly as public_field_name: number, with no name_in_index metadata.
Added a validated per-message next_number cursor; new fields advance from that cursor instead of filling gaps, and older mapping files derive the cursor from their maximum mapped number.
A future proto ingestion adapter can decode records under public field names and pass them through Indexer::RecordPreparer, which owns recursive public-to-index name translation and scalar preparation.
Verification: all 71 elasticgraph-proto_ingestion examples pass with 100% line and branch coverage; lint, spelling, and Steep are green.

Update Aug 8, 12:35 CDT: Addressed the current review feedback.

Messages, oneof alternatives, and enums now allocate exclusively from persisted next_number cursors; cursor-less entries are invalid.
Removed name_in_index from protobuf wire mappings, reserve removed fields and enum values in generated protos, and validate the sidecar with JSON Schema.
Adopted from_parsed_yaml, FromYamlFile, and to_dumpable_hash, reduced allocation mutation, and grouped the mapping specs by behavior.
Eliminated the artifact-specific extension by making schema-artifact preambles configurable.
Verification: all 58 elasticgraph-proto_ingestion examples pass with 100% line and branch coverage; all 1,583 elasticgraph-schema_definition examples pass with 100% coverage; lint and Steep are green.

Update Aug 8, 12:59 CDT: Simplified JSON Schema validation coverage.

Reduced malformed-artifact coverage to one representative cursor-less mapping while retaining the separate cross-field consistency tests.
Verification: focused mapping specs and lint pass; the complete stacked suite has 84 examples with 100% line and branch coverage.

Update Aug 8, 13:57 CDT: Rebased the full four-PR stack onto main at 3256b4c.

Verification: 84 proto-ingestion specs and 1,583 schema-definition specs pass with 100% line and branch coverage; lint and Steep are green.

Update Aug 14, 11:14 CDT: Addressed final non-blocking review feedback.

Deduplicated protobuf number schemas, centralized missing-mapping defaults, and delegated the schema facade methods with Forwardable.
Tightened RBS types and clarified enum allocation cursor names.
Ambiguous multi-source field renames now raise an actionable error instead of silently choosing a mapped historical name.
Verification: all 59 proto-ingestion examples and 1,583 schema-definition examples pass with 100% line and branch coverage; lint, Steep, and documentation coverage are green.

## Why - continue the pluggable ingestion serializer proposal after pulling JSON Schema into its own gem - revive the earlier protobuf prototype from #1056 on top of the new serializer extension points ## What - fill in the `elasticgraph-proto_ingestion` extension gem with core generation: `schema_artifacts:dump` emits a `proto3` `schema.proto` covering the schema's indexed types - map built-in ElasticGraph scalars to proto types, with `t.protobuf type:` for custom scalars (resolved via `type_ref.with_reverted_override` so built-ins renamed with `type_name_overrides` keep working) - generate messages for object/interface/union types and enums (with a zero-valued `*_UNSPECIFIED` entry), escaping proto reserved words and wrapping lists of lists so the output stays valid - keep `schema.proto` on public GraphQL field names; validate proto package names - hold extension state on a `ProtoIngestionState` container behind a single `proto_ingestion_state` reader (matching #1281) Field and enum value numbers are assigned sequentially in definition order in this PR; the stacked follow-up adds the `proto_field_numbers.yaml` sidecar that keeps them wire-stable across schema evolution. ## Stacked follow-ups 1. this PR — core `schema.proto` generation 2. wire-stable field/enum value numbers via a `proto_field_numbers.yaml` sidecar 3. `syntax: :proto2` support and custom file-level `headers:` 4. #1286 — enum value sourcing from existing proto enums + external proto type references ## Verification - `script/run_gem_specs elasticgraph-proto_ingestion` (100% line + branch coverage at this commit) - `script/type_check`, `script/lint`, `script/spellcheck` - `script/quick_build` green at the stack head (whose tree is identical to the previously reviewed single-PR revision) ## References - #1059 - #1056 - #1079 ## Update — 2026-07-10 - The current stack is #1080 → #1304 → #1306 → #1305 → #1286. - Proto extension state now uses a mutable Struct, and keyword package-name segments are validated without being rewritten. - Lists of lists now raise an actionable schema error instead of generating wrapper messages; this supersedes the earlier wrapping note above. --- ## Update — 2026-07-15 - Interface and union messages now wrap concrete subtype messages in a `oneof`, matching the JSON Schema `oneOf` representation. - Concrete subtype messages omit the redundant `__typename` discriminator. - Proto type rendering is now stateless: the generator selects the reachable type graph up front, then each extended type renders itself without mutating shared traversal state. - No-block extension coverage now completes the definitions so the fixture remains valid under CI GraphQL-schema validation. --- ## Update — 2026-07-19 - Replaced keyword suffixing with fully qualified local message and enum references, preserving source type and field names while disambiguating contextual protobuf words and built-in scalar names. - Removed the now-unnecessary keyword collision tracking and verified a generated schema containing contextual names with `protoc` 35.1.

@myronmarston

Today, every schema artifact is a pure function of the schema definition: the schema artifacts directory can safely be deleted and regenerated at any time. In #1304 (comment), @myronmarston pointed out that `proto_field_numbers.yaml` doesn't fit that model--it's an _input_ to `schema.proto` generation, not a pure output--and suggested treating it as part of the schema definition, stored as a sibling of `path_to_schema`, rather than as a schema artifact. For an extension to maintain a file there, schema definition state needs to know where the schema definition lives. `RakeTasks` already knows (it's how the schema gets loaded) but never passed it into the API. This PR stores `path_to_schema` on `SchemaDefinition::State`, allowing extensions to access it without expanding `SchemaArtifactManager`'s dependencies. Nothing in core uses the value yet, so there are no behavior changes to the core artifacts. #1304 builds on this to relocate `proto_field_numbers.yaml`. ## Update — 2026-07-29 After review, the path is carried by schema definition state rather than an extension-only `SchemaArtifactManager` instance variable. The manager continues to accept the narrower schema-definition results object.

myronmarston

Not done reviewing but wanted to submit my feedback so far.

myronmarston

not done but submitting my feedback so far

Field and enum value numbers were assigned sequentially in definition order, so reordering or removing a field renumbered everything after it — breaking wire compatibility with previously serialized data. `schema_artifacts:dump` now reads and writes a `proto_field_numbers.yaml` sidecar artifact: - Existing numbers stay fixed even if field order changes; new fields get the lowest unused numbers. - `schema.proto` keeps the public GraphQL field names while the sidecar stores private `name_in_index` overrides. - A field renamed with `field.renamed_from` reuses its existing number under the new public name. - Enum value numbers are pinned in an `enums` section; removed values keep their numbers reserved so they are never reused (`0` remains the generated `*_UNSPECIFIED` value). - The sidecar is safe to hand-edit but strictly validated: unknown keys, non-integer numbers, out-of-range numbers, and collisions raise clear errors instead of silently reassigning numbers. The parsed mappings are modeled by a `FieldNumberMappings` class that owns conversion to and from the dumped artifact format.

Protobuf field-number state only needs public wire names and stable\nnumbers. Leave public-to-index name translation to record preparation,\nso every field mapping has the same simple integer representation.

Store a per-message allocation cursor beside the stable field mappings.\nNew fields advance from that cursor instead of reclaiming gaps that may\nhave belonged to fields absent from a hand-edited mapping.

myronmarston

LGTM! Left some non-blocking feedback--use your judgement on what to address.

…g formats (#1306) ## Why Mapping `DateTime` to `string` loses the type safety proto offers — a string can be malformed in ways a [`google.protobuf.Timestamp`](https://protobuf.dev/reference/protobuf/google.protobuf/#timestamp) cannot, and proto consumers get language-native timestamp types. For the remaining `string`-typed temporal scalars, the proto type is much wider than the ElasticGraph type, so the expected format deserves documentation at the point of use (per [review discussion](#1080 (comment))). ## What - Map `DateTime` to `google.protobuf.Timestamp`, importing `google/protobuf/timestamp.proto` automatically. A `Timestamp` is a UTC instant, so a publisher's original UTC offset is not preserved; `t.protobuf type: "string"` remains available as an override. - `t.protobuf` gains two options usable by any scalar: - `import:` maps a scalar to an externally defined proto type, emitting the needed `import` statement - `field_comment:` documents the expected format above each generated field - The built-in `string`-typed temporal scalars (`Date`, `LocalTime`, `TimeZone`) use `field_comment:` to document their formats (e.g. `// Must be formatted as an ISO 8601 date, e.g. "2024-11-25".`). Values are still validated at ingestion time, exactly as with JSON ingestion. - Each `protobuf` call replaces the full protobuf configuration, so an override that omits `import:` or `field_comment:` clears the value a prior call set. ## Risk Assessment Low — only affects the unreleased `elasticgraph-proto_ingestion` extension from #1080. ## References - Stacked on #1304 (→ #1080) - Addresses #1080 (comment) and #1080 (comment) ## Update — 2026-07-10 Nested-list wrapper comment propagation was removed after #1080 changed to reject lists of lists. Format comments continue to apply to supported scalar and single-list fields. ## Update — 2026-08-18 Addressed review feedback: - `comment:` is now `field_comment:`, and renders as `//` lines **above** the field rather than trailing the field line. Proto compilers attach leading comments to the code they generate for a field; a trailing comment is usually discarded. This also removed the reason the comment had to be a single line, so a multi-line `field_comment:` is now allowed. - Built-in temporal comments were reworded to read well above the field (e.g. `Must be formatted as an ISO 8601 date, e.g. "2024-11-25".`). - The "each `protobuf` call replaces the full configuration" behavior is now documented and covered in both directions (override that drops the import, and override that drops the field comment). - Enum and object types now answer `protobuf_import` with `nil`, so import rendering no longer greps for scalar types and a non-scalar type can start requiring an import without a change there. - Spec reorganization (moving scalar-related examples out of `schema_spec.rb`) is handled in a follow-up PR at the end of this stack. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

## Why Some consumers need the generated messages to reference proto2 types — `protoc` forbids a proto3 message from referencing a proto2 enum — and language-specific file options (like `java_package`) shouldn't require baking any convention into the gem. ## What `proto_schema_artifacts` gains two options: - `syntax: :proto2` emits a proto2 file instead of the default proto3, labeling every field `optional` or `repeated` - `headers:` injects file-level lines (such as `option` declarations) verbatim as a contiguous section after the `package` declaration ## Risk Assessment Low — only affects the unreleased `elasticgraph-proto_ingestion` extension; both options are opt-in. ## References - Stacked on #1304 (which is stacked on #1080) ## Update — 2026-07-10 This PR is stacked directly on #1306 (→ #1304 → #1080).

jwils requested review from BrianSigafoos-SQ, bsorbo, ellisandrews-toast, jwondrusch, marcdaniels-toast, myronmarston and rossroberts-toast as code owners July 8, 2026 23:21

This was referenced Jul 8, 2026

Support proto2 syntax and custom proto file headers #1305

Merged

Source enum values from external proto enums and reference external enum types #1286

Open

jwils force-pushed the joshuaw/proto-field-numbers branch from 67b8198 to f62a859 Compare July 8, 2026 23:40

jwils mentioned this pull request Jul 9, 2026

Map DateTime to google.protobuf.Timestamp and document temporal string formats #1306

Merged

jwils force-pushed the joshuaw/proto-field-numbers branch 2 times, most recently from 90e2af5 to 616dbfd Compare July 9, 2026 18:13

jwils force-pushed the joshuaw/proto-field-numbers branch from 616dbfd to 74011e5 Compare July 10, 2026 15:44

jwils mentioned this pull request Jul 10, 2026

Generate a protobuf schema artifact from ElasticGraph schemas #1080

Merged

jwils force-pushed the joshuaw/proto-field-numbers branch 2 times, most recently from 075e749 to 8d6e03f Compare July 11, 2026 13:53

jwils force-pushed the joshuaw/proto-field-numbers branch from 8d6e03f to 0a476c1 Compare July 11, 2026 13:59

jwils force-pushed the joshuaw/proto-field-numbers branch 2 times, most recently from 7a57c87 to 26fe805 Compare July 16, 2026 14:21

jwils force-pushed the joshuaw/proto-field-numbers branch from 26fe805 to 8803f5e Compare July 17, 2026 15:05

jwils force-pushed the joshuaw/proto-field-numbers branch from 156369c to ec29566 Compare July 25, 2026 15:30

jwils changed the base branch from main to joshuaw/artifact-manager-schema-path July 25, 2026 15:30

jwils force-pushed the joshuaw/proto-field-numbers branch from ec29566 to e8b37a2 Compare July 29, 2026 19:17

Base automatically changed from joshuaw/artifact-manager-schema-path to main July 31, 2026 03:25

jwils force-pushed the joshuaw/proto-field-numbers branch 5 times, most recently from 4b5acdf to bbe754f Compare August 6, 2026 15:54

myronmarston requested changes Aug 7, 2026

View reviewed changes

jwils force-pushed the joshuaw/proto-field-numbers branch from bbe754f to 92bff82 Compare August 7, 2026 16:20

myronmarston requested changes Aug 8, 2026

View reviewed changes

jwils added 5 commits August 8, 2026 13:55

Keep index names out of protobuf mappings

39a62ec

Protobuf field-number state only needs public wire names and stable\nnumbers. Leave public-to-index name translation to record preparation,\nso every field mapping has the same simple integer representation.

Persist next protobuf field numbers

0d979c2

Store a per-message allocation cursor beside the stable field mappings.\nNew fields advance from that cursor instead of reclaiming gaps that may\nhave belonged to fields absent from a hand-edited mapping.

Address protobuf numbering review feedback

a904dfc

Simplify JSON schema validation coverage

11fc5c1

jwils force-pushed the joshuaw/proto-field-numbers branch from 0ddfe16 to 11fc5c1 Compare August 8, 2026 18:57

myronmarston approved these changes Aug 14, 2026

View reviewed changes

Polish protobuf field number mappings

22dcffe

jwils merged commit 0fbefff into main Aug 15, 2026
22 checks passed

jwils deleted the joshuaw/proto-field-numbers branch August 15, 2026 21:29

jwils mentioned this pull request Aug 15, 2026

Use Buf to guard protobuf schema compatibility #1352

Draft

jwils mentioned this pull request Aug 19, 2026

Generalize indexing schema versions #1284

Draft

Uh oh!

Conversation

jwils commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Risk Assessment

References

Update — 2026-07-15

Update — 2026-07-29

Update — 2026-08-07

Uh oh!

myronmarston left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

myronmarston left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

myronmarston left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jwils commented Jul 8, 2026 •

edited

Loading