Skip to content

Commit 09252f7

Browse files
fix(wire-shape): validator now discovers unannotated Jackson fields
The Java validator's source-discovery only saw fields with @JsonProperty(...) annotations. Many Java SDK POJOs declare fields without the annotation and rely on Jackson's default name-mapping (which works correctly when the Java field name matches the wire key — `id`, `threshold`, `message`, `name`, etc.). The validator under-counted SDK fields in those cases. Three changes to lib.py: - _FIELD_DECL_RE matches plain field declarations: modifier-prefix + type-expression + field-name + `;`/`=`. Captures the modifier sequence so the next change can post-filter. - Filter out declarations whose modifier sequence contains `static` (those are class-level constants — never wire-serialized). - _extract_types_from_java now collects both annotated (@JsonProperty) AND plain field decls. Each plain field's "claim" on a preceding @JsonProperty within a 250-char lookback window is consumed — whichever annotation/field pair is closest. Annotation wins on the wire-name when a field is annotated; field name is used directly when the field is unannotated. Synthetic tests cover: - Mixed annotated + unannotated POJO (BudgetAlert pattern) - `private static final String CONST = "..."` excluded - Methods (have `(`) excluded - Generics like `Map<String, Object>` and `List<String>` - Record params with `@JsonProperty` - Class with both constants and fields Effect on Java SDK baseline: - registered_types: 70 → 73 (3 types newly visible; previously had no @JsonProperty annotations so the validator skipped them) - per_type_drift: 37 → 39 (newly visible types surfaced 2 more drift entries; previously hidden coverage gaps) The drift increase is the validator working correctly for the first time — the previous count was an under-measurement, not a real "no drift" signal.
1 parent 489096a commit 09252f7

2 files changed

Lines changed: 122 additions & 54 deletions

File tree

scripts/wire_shape/lib.py

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,34 @@ class whose body contains them.
5454
)
5555
_JSON_PROPERTY_RE = re.compile(r'@JsonProperty\(\s*"([^"]+)"\s*\)')
5656

57+
# Matches a Java instance field declaration that uses Jackson's
58+
# default field-name mapping (no @JsonProperty alias). The pattern:
59+
# - the leading modifier sequence (group 1) — captured so we can
60+
# post-filter declarations that include `static` (constants
61+
# don't go on the wire)
62+
# - a type expression: identifier with optional `.qualifier`,
63+
# optional `<...>` generics, optional `[]` array
64+
# - the field name (group 2)
65+
# - terminated by `;` or `=` (initializer)
66+
#
67+
# `static` IS allowed in the modifier set (otherwise the regex would
68+
# anchor at `final` in `private static final ...`); we drop the
69+
# match in code when `static` appears in group 1.
70+
# Methods are excluded by the absence of `(` before the terminator.
71+
_FIELD_DECL_RE = re.compile(
72+
r"((?:(?:private|protected|public|static|final|transient|volatile)\s+)+)"
73+
r"(?:[\w.]+(?:\s*<[^<>;]*>)?(?:\s*\[\s*\])?)\s+"
74+
r"(\w+)\s*[;=]"
75+
)
76+
# Cap on lookback distance from a field declaration to a
77+
# preceding @JsonProperty(...) annotation: if the most recent
78+
# annotation sits within this many characters of the field's
79+
# declaration position, the field is considered "annotated" and we
80+
# defer to the @JsonProperty value rather than the field name.
81+
# 250 chars covers an annotation on the line immediately above plus
82+
# JavaDoc and other annotations like @JsonInclude in between.
83+
_JSON_PROPERTY_LOOKBACK = 250
84+
5785

5886
def load_all_schemas(spec_dir: Path) -> tuple[
5987
dict[str, list[str]],
@@ -258,12 +286,58 @@ def _extract_types_from_java(content: str) -> dict[str, list[str]]:
258286
# blanked range (string literal, comment, text block) — the
259287
# cleaner replaces those with whitespace, so cleaned[m.start()]
260288
# is a space rather than the original `@`.
261-
props: list[tuple[int, str]] = [
289+
annot_props: list[tuple[int, str]] = [
262290
(m.start(), m.group(1))
263291
for m in _JSON_PROPERTY_RE.finditer(content)
264292
if m.start() < len(cleaned) and cleaned[m.start()] == "@"
265293
]
266294

295+
# Plain (unannotated) field declarations. Jackson defaults to
296+
# using the field name as the wire key when @JsonProperty is
297+
# absent. Without this discovery, the wire-shape gate
298+
# under-counts SDK fields wherever the SDK relies on Jackson's
299+
# default name-mapping (which is correct for fields whose Java
300+
# name already matches the wire — `id`, `threshold`, `message`,
301+
# etc.). Match on the cleaned text so commented-out or in-string
302+
# field-shaped patterns don't false-fire. Skip declarations
303+
# whose modifier sequence contains `static` — those are
304+
# class-level constants, never serialized.
305+
field_decls = [
306+
fm for fm in _FIELD_DECL_RE.finditer(cleaned)
307+
if "static" not in fm.group(1)
308+
]
309+
310+
# Build the merged property stream. Each entry is (position,
311+
# wire_name). For plain field decls, "absorb" any @JsonProperty
312+
# whose position falls within the look-back window — that pair
313+
# is annotated and the annotation's wire name takes precedence.
314+
consumed_annot_indices: set[int] = set()
315+
plain_props: list[tuple[int, str]] = []
316+
for fm in field_decls:
317+
field_pos = fm.start()
318+
field_name = fm.group(2)
319+
# Find the latest annotation that hasn't been consumed and is
320+
# within the lookback window.
321+
annotated = False
322+
for idx, (ap, _) in enumerate(annot_props):
323+
if idx in consumed_annot_indices:
324+
continue
325+
if ap >= field_pos:
326+
break
327+
if field_pos - ap <= _JSON_PROPERTY_LOOKBACK:
328+
consumed_annot_indices.add(idx)
329+
annotated = True
330+
# Don't break — multiple annotations can stack
331+
# (e.g. @JsonInclude + @JsonProperty); keep marking
332+
# them consumed so they don't double-attribute.
333+
if not annotated:
334+
plain_props.append((field_pos, field_name))
335+
336+
# The full property stream is annotation-derived names + plain
337+
# field names, ordered by source position so the depth walker
338+
# attributes them in declaration order.
339+
props: list[tuple[int, str]] = sorted(annot_props + plain_props)
340+
267341
# Walk the cleaned source. Two queues: decls already seen but
268342
# whose body `{` hasn't been hit yet (pending), and active type
269343
# frames (stack). Annotations attribute to the innermost pending

tests/fixtures/wire-shape-baseline.json

Lines changed: 47 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -202,23 +202,14 @@
202202
"spec_only": []
203203
},
204204
"Budget": {
205-
"sdk_only": [],
205+
"sdk_only": [
206+
"enabled"
207+
],
206208
"spec_only": [
207-
"id",
208-
"name",
209209
"org_id",
210-
"period",
211-
"scope",
212210
"tenant_id"
213211
]
214212
},
215-
"BudgetStatus": {
216-
"sdk_only": [],
217-
"spec_only": [
218-
"budget",
219-
"percentage"
220-
]
221-
},
222213
"CancelPlanResponse": {
223214
"sdk_only": [
224215
"message"
@@ -260,16 +251,8 @@
260251
"organization_id"
261252
],
262253
"spec_only": [
263-
"action",
264-
"category",
265-
"description",
266-
"enabled",
267-
"name",
268-
"pattern",
269254
"priority",
270-
"severity",
271-
"tags",
272-
"tier"
255+
"tags"
273256
]
274257
},
275258
"CreateWorkflowResponse": {
@@ -283,18 +266,15 @@
283266
},
284267
"DynamicPolicy": {
285268
"sdk_only": [
269+
"category",
286270
"created_at",
287271
"organization_id",
272+
"priority",
273+
"tier",
274+
"type",
288275
"updated_at"
289276
],
290-
"spec_only": [
291-
"actions",
292-
"conditions",
293-
"description",
294-
"enabled",
295-
"id",
296-
"name"
297-
]
277+
"spec_only": []
298278
},
299279
"DynamicPolicyInfo": {
300280
"sdk_only": [
@@ -427,6 +407,15 @@
427407
"workflow_execution_id"
428408
]
429409
},
410+
"PolicyAction": {
411+
"sdk_only": [
412+
"value"
413+
],
414+
"spec_only": [
415+
"config",
416+
"type"
417+
]
418+
},
430419
"PolicyInfo": {
431420
"sdk_only": [
432421
"code_artifact",
@@ -446,7 +435,9 @@
446435
]
447436
},
448437
"PolicyOverride": {
449-
"sdk_only": [],
438+
"sdk_only": [
439+
"active"
440+
],
450441
"spec_only": [
451442
"enabled_override",
452443
"id"
@@ -462,8 +453,7 @@
462453
"change_summary",
463454
"id",
464455
"policy_id",
465-
"snapshot",
466-
"version"
456+
"snapshot"
467457
]
468458
},
469459
"ResumePlanResponse": {
@@ -482,19 +472,8 @@
482472
"StaticPolicy": {
483473
"sdk_only": [],
484474
"spec_only": [
485-
"action",
486-
"category",
487-
"description",
488-
"enabled",
489-
"id",
490-
"name",
491-
"override",
492-
"pattern",
493475
"policy_id",
494-
"priority",
495-
"severity",
496-
"tier",
497-
"version"
476+
"priority"
498477
]
499478
},
500479
"StepGateRequest": {
@@ -517,38 +496,50 @@
517496
"metadata"
518497
]
519498
},
499+
"UpdateStaticPolicyRequest": {
500+
"sdk_only": [
501+
"category"
502+
],
503+
"spec_only": [
504+
"priority",
505+
"tags"
506+
]
507+
},
520508
"UsageBreakdown": {
521509
"sdk_only": [
510+
"period",
522511
"period_end",
523512
"period_start"
524513
],
525-
"spec_only": [
526-
"items"
527-
]
514+
"spec_only": []
528515
},
529516
"UsageBreakdownItem": {
530517
"sdk_only": [],
531518
"spec_only": [
532-
"group_by",
533-
"percentage"
519+
"group_by"
534520
]
535521
},
536522
"UsageRecord": {
537-
"sdk_only": [],
523+
"sdk_only": [
524+
"timestamp"
525+
],
538526
"spec_only": [
539527
"created_at",
540528
"error_message",
541-
"id",
542529
"latency_ms",
543-
"model",
544-
"provider",
545530
"success",
546531
"team_id",
547532
"tenant_id",
548533
"user_id",
549534
"workflow_id"
550535
]
551536
},
537+
"UsageSummary": {
538+
"sdk_only": [
539+
"period"
540+
],
541+
"spec_only": []
542+
},
552543
"WorkflowStatusResponse": {
553544
"sdk_only": [],
554545
"spec_only": [
@@ -608,12 +599,14 @@
608599
"PlanVersionEntry",
609600
"PlanVersionsResponse",
610601
"PlatformCapability",
602+
"PolicyAction",
611603
"PolicyEvaluationResult",
612604
"PolicyInfo",
613605
"PolicyMatch",
614606
"PolicyMatchInfo",
615607
"PolicyOverride",
616608
"PolicyVersion",
609+
"PricingInfo",
617610
"RateLimitInfo",
618611
"ResumeFromCheckpointResponse",
619612
"ResumePlanResponse",
@@ -627,6 +620,7 @@
627620
"ToolContext",
628621
"UpdatePlanRequest",
629622
"UpdatePlanResponse",
623+
"UpdateStaticPolicyRequest",
630624
"UpdateWebhookRequest",
631625
"UsageBreakdown",
632626
"UsageBreakdownItem",

0 commit comments

Comments
 (0)