~bigbes/tarantool

tarantool-protobuf

51f196338ff5ff64768ab451182c7b04d79c840b — Eugene Blikh 3 months ago 343e4ba
docs: handoff brief for the pb.text.decode slice

Self-contained brief targeting the 408 proto3 TextFormatInput tests
still skipped after the encoder + SGROUP work. Covers existing
surface (text.lua encode-only, parser.lua schema parser, conformance
dispatch), grammar buckets the suite exercises (10 categories with
upstream test counts), files to touch, LuaJIT/cdata/box.NULL
conventions, and a verifying-locally checklist. Suggested
recursive-descent shape mirrors runtime/pb/parser.lua's cursor
mechanics.

No code changes; positions the next slice without committing to a
specific implementation schedule.
1 files changed, 254 insertions(+), 0 deletions(-)

A docs/text_format_parser_brief.md
A docs/text_format_parser_brief.md => docs/text_format_parser_brief.md +254 -0
@@ 0,0 1,254 @@
# Text-format parser brief — `pb.text.decode`

Handoff doc for the next slice of text-format conformance work. Goal:
turn the 408 proto3 `TextFormatInput.*` tests we currently `skipped`
into passes. Read this end-to-end before opening files.

## What exists

- `runtime/pb/text.lua` — encode-only proto3 text-format printer.
  Walks descriptor fields and emits `name: value` / `name { ... }`,
  honors `_unknown_fields` under `opts.print_unknown_fields`. WKT
  emitters for Timestamp/Duration/Struct/Value/ListValue/Any/FieldMask/
  Empty/Wrappers all hook in via `desc.text(t, buf, depth)`.
- `cmd/conformance/core.lua` — TEXT input still returns
  `{skipped = 'text-format input not supported'}`. PB and JSON inputs
  dispatch into `pb.decode` / `pb.json.decode`. TEXT output is wired.
- `runtime/pb/parser.lua` — pure-Lua proto3 *schema* parser (different
  artifact; parses `.proto` files into AST). Useful only as a style
  reference for hand-written Lua recursive-descent.
- `test/text_test.lua` — encoder tests parameterized over both codegen
  modes (`full`, `runtime`). Pattern to mirror for decoder tests.
- `test/conformance_test.lua` — unit regressions that pin each upstream
  conformance case to a `core.handle_request` call. Cheaper than the
  Docker round-trip for the inner dev loop.

## Goal

Add `pb.text.decode(desc, text) -> table` (mirroring
`pb.text.encode(desc, t, opts)`) so:

1. `cmd/conformance/core.lua` stops skipping `text_payload` inputs,
   instead calling `pb.text.decode(desc, req.text_payload)` and feeding
   the result into the existing output dispatch.
2. The proto3 text suite under
   `test/conformance/known_failures_text.txt` is empty *after*
   parser-level conformance is closed (some Recommended-only edge
   cases may legitimately fail and need an entry — fine, mark them).
3. `test/conformance_test.lua` grows one regression per scenario
   bucket (see "Grammar coverage" below) so the inner loop doesn't
   require Docker.

Encoder is already complete for the conformance corpus; do **not**
expand encoder scope unless the conformance probe shows a specific
gap. The only encoder polish in scope: emit textproto-style list
short-form `[a, b, c]` for repeated scalars when `opts.list_short =
true`, so decoded text round-trips cleanly through encode. Default
stays one-line-per-element (the form the current corpus uses).

## Acceptance criteria

- `make test` stays green; the existing 509 unit tests don't regress.
- New unit tests under `test/text_decode_test.lua` cover each grammar
  item in the table below, parameterized over both `full` and
  `runtime` modes against the `hello.Person` and `proto3.TestAllTypes`
  descriptors.
- New regressions under `test/conformance_test.lua` in a
  `core.text_input_*` cluster, one per scenario bucket, pinned with
  byte-exact `text_payload` strings cribbed from the upstream
  conformance source (see "How to extract test payloads" below).
- Docker conformance under default `just conformance` settings:
  binary+JSON suite stays at 1478 ✓; text-format suite climbs from
  8 ✓ / 426 skipped → ~416 ✓ / ~18 skipped (the residual ~18 are
  proto2 message-type skips, unaffected by this work).
- `runtime/pb/text.lua` decoder respects the same conventions as the
  rest of the runtime: LuaJIT `int64_t`/`uint64_t` cdata for 64-bit
  ints; `box.NULL` for the `Value` WKT's `null_value`; `ipairs` /
  `for i = 1, #t` on any hot path (`pairs()` over a hash NYIs the JIT
  trace per CLAUDE.md).

## Files to touch

```
runtime/pb/text.lua             — add decode() and helpers; keep encode untouched
cmd/conformance/core.lua        — drop the text_payload skip, call pb.text.decode
test/text_decode_test.lua       — new, parameterized over modes
test/conformance_test.lua       — add a core.text_input_<bucket> cluster
test/conformance/known_failures_text.txt
                                — empty (or only Recommended residuals)
PLAN.md                         — bump M7 baseline: 8 ✓ → ~416 ✓
runtime/pb/init.lua             — no surface change; pb.text already exposed
```

No codegen-side changes (`cmd/protoc-gen-tarantool/...`) — decode is
descriptor-driven, not inlined.

## Grammar coverage (from a `--verbose` skip-name extract)

The conformance suite stresses these grammar features. Cover each
under both the unit-test set and the conformance regression cluster.
Counts are upstream-test pair counts (input is duplicated × output
formats).

| Bucket | Count | Grammar feature |
|---|---:|---|
| Number literals | ~24 | decimal, hex (`0x…`), octal (`0…` prefix), trailing `f`/`F` for floats, signed exponents (`1e-50`, `1e9999`); `Int{32,64}/Uint{32,64}FieldMaxValue{Octal,Hex,}` all radixes |
| Float specials | ~50 | `inf`, `infinity`, `nan` in arbitrary casings (`NaN`, `iNF`, `INFINITY`); oversize exponents parse to ±inf; `Neg{Float,Double}LargeNegativeExponentParsesAsNegZero` (`-1e-50` → `-0.0`) |
| String/bytes literals | ~24 | `"…"` and `'…'`; escapes `\a \b \f \n \r \t \v \? \\ \' \"`; octal `\341`, hex `\xc0`, short-unicode `ሴ`, long-unicode `\U00010437`; adjacent-literal concat (`"a" "b"` → `"ab"`); UTF-8 validation for `string` fields (reject surrogates, overlongs); `bytes` accepts any byte |
| Aggregates | ~30 | `field: { ... }` and `field: < ... >` (legacy angle-bracket); fields separated by `;`, `,`, or whitespace; trailing `;` accepted; **multiple trailing** `;;` accepted (a corner case the suite tests) |
| Repeated short-form lists | ~30 | `repeated_int32: [123, 456]`, with `,` separator (and trailing `;` after the list). `[a b]` space-only is **invalid**; `[a; b]` semicolons inside lists is **invalid** |
| ReservedFieldName | 20 | any value shape after a reserved-field name (literal, list, message, group-style) must parse and be **silently dropped** — not produce parse_error |
| Map entries | ~8 | `map_<k>_<v> { key: K value: V }` aggregate form, one entry per occurrence |
| Any-typed inline | ~12 | `[type.googleapis.com/<full.name>] { ... }` resolves to an Any with the inner message bytes; the URL syntax inside square brackets is the type-tag |
| Enums | ~4 | accept enum-by-name and enum-by-integer; unknown enum *name* must reject (proto3 rule) |
| Map key types | ~3 | bool keys (`key: true`), int keys, string keys |

A scenario worth flagging: `ListSeparatorMissingIsOneValue_*` —
`field: [123]` followed by a *separate* `field: [456]` produces a
two-element repeated list. Don't collapse the two entries; they're
distinct list-shorts that each append.

## How to extract test payloads

For unit-test regressions, paste the byte-exact `text_payload` from
the upstream conformance source rather than reconstructing:

```bash
# All proto3 text-input test names + their payloads:
docker run --rm -v "$(pwd):/work" -w /work \
    --entrypoint conformance_test_runner <image> \
    --enforce_recommended --verbose \
    --failure_list test/conformance/known_failures.txt \
    --text_format_failure_list test/conformance/known_failures_text.txt \
    /usr/bin/tarantool cmd/conformance-runner.lua 2>&1 \
    | grep '^SKIPPED.*Proto3.*TextFormatInput'
```

Each line carries `test=<name>, request=goo.gle/debugproto <serialized
ConformanceRequest including text_payload>`. The payload after
`text_payload: "` is the input text (C-escaped); unescape it for the
Lua string literal.

Alternatively, `--test <name> --debug` dumps the octal-escaped request
for a single test, which is easier to copy verbatim.

## Constraints / conventions

- **No `pairs()` on hot paths** — the proto3 spec lets fields appear
  in any order, but the parser's outer loop is `while pos < len do`
  over the input string, not iteration over a hash. Field lookup
  inside the loop must be `field_by_name[…]` (already O(1) hash
  lookup, but only one per field-occurrence, not per byte). Map
  fields are the one accepted exception per CLAUDE.md.
- **int64 family stays cdata.** `decode_uint64_token` returns
  `ffi.cast('uint64_t', …)`, not a Lua number. The runtime's `wire.lua`
  has `to_uint64` / `to_int64` helpers — use them for big integer
  literals (esp. `0xFFFFFFFFFFFFFFFF` and `18446744073709551615`).
- **`box.NULL` for nulls.** When the parser encounters `null_value:
  NULL_VALUE` inside a Value WKT, emit `box.NULL`, not `nil` / not a
  custom sentinel — `runtime/pb/wkt.lua` already does this convention.
- **Numeric-field-id mode is optional** — protoc's
  `AllowFieldNumber(true)` is what lets the harness re-parse our
  output. For the parser, accept numeric field IDs by default since
  the conformance harness emits some scenarios that way (e.g.
  unknown-field text round-trips). Skip silently if the ID isn't in
  the schema; same shape as `reserved` handling.
- **Token-recursion depth limits**: textproto recommends rejecting
  deeply nested messages. A 100-level cap mirrors mainline protoc
  and stops pathological inputs from blowing the Lua stack.
- **Reuse `runtime/pb/wire.lua` numeric edge logic.** Don't
  re-derive int32-overflow rules in the parser — produce the value
  as a uint64 cdata and feed through the same truncation helpers the
  binary decoder uses (`wire.varint_to_int32`, etc.).

## Suggested implementation outline

Recommend recursive-descent, not a generated parser. The grammar is
small enough that a hand-written tokenizer + parser stays under ~600
LOC. Mirror `runtime/pb/parser.lua` (the `.proto` schema parser) for
buffer/cursor mechanics: a `lexer` table with `pos`, `src`, `peek()`,
`expect(tok)`, plus skip-whitespace-and-comments helpers (textproto
supports `#` line comments; the conformance corpus tests at least one
comment scenario).

Top-down phases:

1. **Lexer** — number, string-literal (with all escape variants),
   identifier, bracketed-Any URL, `:`, `{`, `}`, `<`, `>`, `[`, `]`,
   `,`, `;`. Single function returning `(token_kind, token_value)`
   and advancing `pos`. Skips whitespace + `#...\n` comments.
2. **Value parser** — given a field's `kind` + `proto_type`, consume
   one value from the lexer. For scalars: type-specific token form
   validation (e.g. floats accept `inf`/`nan`, ints don't). For
   messages: open `{` or `<`, recurse, close.
3. **Field parser** — consume identifier, optional `:`, value, optional
   trailing `;` or `,`. Look up by name in `desc.field_by_name`. If
   field is `reserved` (need to thread reserved set into descriptors;
   currently absent — see "Schema-side prep" below), parse-and-drop.
   If unknown name and `opts.allow_unknown_fields = false`, error;
   otherwise drop.
4. **Top-level entry** — `M.decode(desc, text)` constructs an empty
   result, loops `field_parser` until input exhausted.

WKTs need explicit overrides on the decode path too (Timestamp parses
RFC3339-ish forms when given as `seconds: N nanos: M`, Duration similar,
Value parses the oneof from token shape: `string_value: "..."` vs
`number_value: 1.5` vs `bool_value: true` vs `list_value { ... }` vs
`struct_value { ... }` vs `null_value: NULL_VALUE`).

### Schema-side prep

To pass `ReservedFieldName` we need to know reserved names. They're in
`FileDescriptorProto.MessageType[].ReservedName` (proto descriptor),
which our `cmd/protoc-gen-tarantool` plugin currently ignores. Plumb
`reserved_names = {…}` onto generated descriptors (both modes) and
expose `desc.is_reserved(name) -> bool` for the parser. Plugin change
+ regen via `make gen`.

## Out of scope

- Proto2 / editions message types. Even after this lands, ~1331
  proto2 tests stay skipped because we don't generate Lua for
  `TestAllTypesProto2`. That's a separate axis: generate proto2
  bindings, register the descriptor in `MESSAGE_REGISTRY`, then ~1331
  more tests light up (most will likely pass since the wire format is
  unchanged proto3-shape, but some proto2-specific shapes —
  extensions, groups as a *first-class* feature, required-field rules
  — need work).
- JSPB. Google-internal; nobody implements.
- TextFormat printer changes beyond the optional `list_short` opt.
- Streaming / incremental parse. The conformance corpus payloads are
  small; one-shot string decode is fine.

## Verifying locally

Inner loop (no Docker):

```bash
make gen && make test         # full luatest suite, ~0.5s
# Spot-check one conformance test:
LUA_PATH="./runtime/?/init.lua;./runtime/?.lua;./examples/expected/?.lua;./examples/expected/?/init.lua;./cmd/?.lua;./cmd/?/init.lua;./test/?.lua;;" \
    .rocks/bin/luatest -v test/text_decode_test.lua
```

Full conformance (5–8 min):

```bash
just conformance              # both suites, both failure lists, --enforce_recommended
```

Single test (fast triage):

```bash
docker run --rm -v "$(pwd):/work" -w /work \
    --entrypoint conformance_test_runner \
    tarantool-protobuf-conformance:latest --enforce_recommended \
    --test Required.Proto3.TextFormatInput.ReservedFieldName \
    --failure_list test/conformance/known_failures.txt \
    --text_format_failure_list test/conformance/known_failures_text.txt \
    /usr/bin/tarantool cmd/conformance-runner.lua
```

When the unit + parity tests are green and `just conformance` exits
zero, update `PLAN.md` with the new baseline and commit on top of
this branch (`text-conformance-output`).