Skip to content

yq: metadata builtins answer from the wrong node after key (line_comment, head/foot_comment, line, column, style) #2763

Description

@newhoggy

Symptom

A mapping entry's metadata lives on its key node. succinctly loses the key
node's cursor at a key stage, so every metadata builtin after key answers
from a no-cursor default instead. Captured just now against pinned yq v4.53.3,
on a: # keyc\n b: 1\n (and "qk": 1 for style):

.a | key | … yq succinctly
line_comment keyc "" wrong — #765's residual
line 1 0 wrong
column 1 0 wrong
style (quoted key) double "" wrong
head_comment / foot_comment the comment "" wrong (#2758)
tag, kind !!str, scalar same already correct

Also .b | key | head_comment on a: 1\n# mid\nb: 2\n is mid in yq, ""
here.

Note this is not only a comments problem — line/column/style are
affected too, and nothing tracked that before. The 0/"" answers are the
no-cursor default: .a | line on the value node is 2 in both, so these
builtins are fine whenever they have a cursor.

Must keep working (currently correct, and a naive fix breaks them):
.a.b | key | path → ["a","b"], .a.b | key | parent | key → "a",
.a.b | key | key → nothing.

Where the wall is

cursor_key (src/jq/eval_generic.rs) finds the field by comparing against
field.value_cursor, returns field.key's display string, and discards
field.key_cursor — the cursor every one of these builtins would need.

Routing, not just the missing read, is what blocks a fix:

  • owned_identity_pipe_applies gates the identity route on
    rest.iter().any(needs_path_context). line_comment and friends do not
    need path context, so ... | key | line_comment never enters that route
    (... | key | path does, which is why path is correct).
  • Widening that gate is not enough either: path_context_walk_split leaves a
    non-empty rest for these pipes, so the walk route intercepts first and
    evaluates the tail from a materialized node with no cursor.

Two designs that are wrong — please don't repeat them

Both were tried in #2758 and reverted; each was caught only by experiment.

  1. Make key return field.key_cursor. Regresses working behaviour:
    key's result deliberately stands at the value node, which is what makes
    .a.b | key | path answer ["a","b"]. That is pinned by captured yq
    behaviour in OwnedIdentityRule::KeyNode's own doc comment and by
    jq_evaluator_parity_tests/jq_path_context_alphabet_tests.

  2. Resolve the key cursor in eval_owned_identity_stages. Correct in
    shape — OwnedIdentity already carries a live base cursor plus a
    key_node flag, and the key cursor is one document_parent()-and-scan
    away — but unreachable, because the pipe never reaches that route (see
    above). Widening the entry gate still loses to the walk route.

Likely shape of a real fix

Change how the walk route evaluates its rest so a ... | key | <metadata>
tail is evaluated from a carried position rather than a materialized node —
i.e. path_context_walk_split/path_context_absent_rest_route's territory,
the #2416 spine. That wants its own design pass; it is not a small patch.

Whatever the mechanism, it should fix all six builtins at once rather than
comments alone — they share the single cause.

Verification notes for whoever takes it

  • The failure mode is silence: a miss answers ""/0, exactly what a test
    asserting "absent" accepts. Assert non-empty values through a full pipe,
    and include the iterating shape ([.[] | key | head_comment]), which routes
    differently from direct navigation.
  • jq_evaluator_parity_tests and jq_path_context_alphabet_tests are the
    regression gate for design 1's failure mode.
  • feat(yq): add head_comment/foot_comment getters (#798) #2758 pins the current wrong answers for head_comment and line_comment
    in head_foot_comment_798::key_node_head_comment_is_not_reachable_yet_798,
    so closing this should update that test rather than delete it.

Related: #765 (closed — capture side landed, this getter residual remained),
#2758, #798.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions