Skip to content

[Collect One 3/60] Protos: forms (P1) - #2631

Closed
gino-m wants to merge 11 commits into
c1from
review/c1/p1-protos-forms
Closed

gino-m wants to merge 11 commits into
c1from
review/c1/p1-protos-forms

Conversation

@gino-m

@gino-m gino-m commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Collect One review series: P1 Protos: forms (3/60)

Tracking issue: #2629 · Stacked on: #2630 (C01) · Reviewer: @rfontanarosa
FYI: @gino-m @shobhitagarwal1612

Context

Collect One is the Kotlin Multiplatform successor to Ground: a single shared codebase for the web console, the mobile app (Android, iOS, browser), and a Spanner-backed user-data service. Forms use ProtoForms, a lossless proto3 encoding of ODK XForms.

This PR is one slice of the frozen wip/baobab tree (collect-one-review-base), landing through 60 stacked PRs into c1 (see #2629). It only adds the files listed below, and it's stacked on C01 (#2630), so the diff shows only this slice.

The proto PRs (P1–P3) import each other, e.g. survey_def.proto imports forms/form_def.proto and data/audit_record.proto. Nothing compiles them until shared/core lands in K1, after all three merge, so please review the schemas as a contract. CI only checks buf format here.

What's in this PR

  • shared/protos/README.md: the mental model behind all the schemas (data tables / map layers = persistent EntityRecords; forms = immutable encounter logs), the package map, and conventions.
  • forms/form_def.proto (groundplatform.v2.forms): FormDef and the XForms model. The model (ModelDef, RecordSchema, PrimaryInstance/SecondaryInstance, FieldBinding with relevance/constraint/calculate/required/readonly, PreloadType). The body (ControlDef, ControlType, GroupDef, RepeatDef, ItemsetDef, ChoiceItem, RangeConfig, GeoConfig, IntentConfig). Events and actions (EventType, ActionDef, e.g. setvalue/setgeopoint). Translations (TranslationCatalog, LocalizedString, MediaRef). ODK Entities (EntityDeclaration, EntityPropertyMapping, EntitySyncMetadata). Submission config. Optional view layout (ViewDef).
  • forms/types.proto: DataType, TypedValue, GeoPoint/GeoTrace/GeoShape.
  • forms/record_instance.proto: reflection-free RecordInstance tree (RecordNode, FieldValue, repeats, RecordMetadata aligned with OpenRosa <orx:meta>).
  • forms/audit_log.proto: ODK client audit log config and events.
  • forms/encrypted_submission.proto: envelope-encryption manifest for encrypted submissions.

Where it fits

ProtoForms is the canonical form representation everywhere. shared/core (K1–K5) converts it to and from XForms XML, TextProto and Proto3 JSON, evaluates it with an XPath 1.0 engine, and runs it in the form engine. Survey definitions (P2) embed FormDef. Submissions (P3) carry RecordInstances.

Not in this PR

  • Survey container, ACLs, organizations and their services → P2. Records, services, library, sync state → P3.
  • The formal model spec (one page per XForms area) → D2.
  • Serializers and round-trip tests that prove XML → proto → XML fidelity → K2/K3.

Suggested reading order

  1. shared/protos/README.md
  2. types.proto, then record_instance.proto
  3. form_def.proto top-down: FormDef → ModelDef → bindings → body controls → entities
  4. audit_log.proto, encrypted_submission.proto

Focus areas

  • XForms compatibility boundary: everything here should be expressible in the ODK XForms spec (including jr:/odk: extensions and Entities) and round-trip through XLSForm → ODK Central → Collect/Enketo. Anything Ground-specific should live in P2/P3 instead.
  • Field numbering and oneof choices that would be painful to change later.
  • Naming and comments: do they cite the spec sections they model?

Safe to skim

  • Long enum comment blocks that restate ODK appearance and preload names.

Size

Source Tests Data Files
1,409 0 0 6

Testing

bash scripts/buf-format.sh --check: PASS. CI: check (ktfmt, buf). Compilation and round-trip tests arrive with K1/K2.

@gino-m gino-m added the collect-one Collect One review series (#2629) label Oct 19, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces the canonical Protocol Buffer schemas and gRPC service definitions for the Collect One platform, establishing a structured data model for forms, surveys, and offline-first data synchronization. The feedback highlights several opportunities to improve schema correctness, type safety, and documentation. Key recommendations include renaming misleading 'base64_' prefixed fields that store raw binary bytes, using 'google.type.Date' instead of 'google.protobuf.Timestamp' for calendar dates to avoid timezone shifting bugs, and leveraging 'optional' fields to enable presence tracking for defaultable values like delimiters and geospatial coordinates. Additionally, it is advised to group mutually exclusive fields into a 'oneof' block and document OS-level privacy restrictions on sensitive hardware identifiers.

Comment thread shared/protos/forms/encrypted_submission.proto Outdated
Comment thread shared/protos/forms/encrypted_submission.proto Outdated
Comment thread shared/protos/forms/record_instance.proto Outdated
Comment thread shared/protos/forms/form_def.proto
Comment thread shared/protos/forms/form_def.proto Outdated
Comment thread shared/protos/forms/types.proto Outdated
Comment thread shared/protos/forms/record_instance.proto Outdated
@gino-m
gino-m requested a review from rfontanarosa October 19, 2026 17:59
gino-m added a commit that referenced this pull request Oct 20, 2026
…key and encrypted_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
gino-m added a commit that referenced this pull request Oct 20, 2026
…ed_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
gino-m added a commit that referenced this pull request Oct 20, 2026
Rename encrypted_submission bytes fields to encrypted_key and encrypted_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
@gino-m

gino-m commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator Author

Heads-up: encrypted form submissions are being removed from Collect One (no code used them, and the server needs to read submissions). The change for this slice is in #2635, which merges into this PR's branch. Please review it alongside this PR.

Comment thread shared/protos/forms/encrypted_submission.proto Outdated
gino-m added a commit that referenced this pull request Oct 20, 2026
…ed_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
@gino-m
gino-m force-pushed the review/c1/p1-protos-forms branch from f50601c to bceb8fe Compare October 20, 2026 08:32
gino-m added a commit that referenced this pull request Oct 20, 2026
Rename encrypted_submission bytes fields to encrypted_key and encrypted_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
gino-m added a commit that referenced this pull request Oct 20, 2026
gino-m added a commit that referenced this pull request Oct 20, 2026
gino-m added a commit that referenced this pull request Oct 21, 2026
…ed_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
@gino-m
gino-m force-pushed the review/c1/p1-protos-forms branch from 731b8c6 to 438c98a Compare October 21, 2026 07:22
gino-m added a commit that referenced this pull request Oct 21, 2026
Rename encrypted_submission bytes fields to encrypted_key and encrypted_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
@gino-m
gino-m removed the request for review from andreia-ferreira October 21, 2026 19:46
Comment thread shared/protos/forms/audit_log.proto
Comment thread shared/protos/README.md Outdated
gino-m added a commit that referenced this pull request Oct 22, 2026
…ed_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
@gino-m
gino-m force-pushed the review/c1/p1-protos-forms branch from 14992ca to a1e183e Compare October 22, 2026 19:12
gino-m added a commit that referenced this pull request Oct 22, 2026
Rename encrypted_submission bytes fields to encrypted_key and encrypted_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
Base automatically changed from review/c1/c01-repo-scaffolding to c1 October 23, 2026 09:13
gino-m and others added 11 commits October 23, 2026 09:13
ProtoForms (`groundplatform.v2.forms`): XForms data model in proto3.

Sliced from collect-one-review-base (wip/baobab @ b7fd591). Tracking: #2629.
…ed_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@gino-m
gino-m force-pushed the review/c1/p1-protos-forms branch from a1e183e to 2a91e77 Compare October 23, 2026 09:13
gino-m added a commit that referenced this pull request Oct 23, 2026
Rename encrypted_submission bytes fields to encrypted_key and encrypted_signature

Both fields are proto bytes, so they hold raw bytes, not base64 text; the base64_
prefix was misleading. Field numbers are unchanged (wire-compatible).
Review: #2631 (comment)
and #discussion_r4316641039.
@gino-m gino-m closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

collect-one Collect One review series (#2629)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants