Skip to content

refactor: remove duplicate structured field helper - #183

Merged
sromoam merged 1 commit into
awslabs:devfrom
hugosmoreira:fix/163-remove-dead-field-helper
Aug 6, 2026
Merged

sromoam merged 1 commit into
awslabs:devfrom
hugosmoreira:fix/163-remove-dead-field-helper

Conversation

@hugosmoreira

Copy link
Copy Markdown
Contributor

Issue #, if available:

Closes #163

Description of changes:

Removes the unused FieldHelper.is_structured_field_type implementation and the imports that only supported it.

ConfigurationHelper.is_structured_field_type remains the single structured-field classifier used by the model pipeline. Unlike the removed duplicate, it also handles typing.Union and PEP 604 union annotations. This is a maintenance cleanup with no runtime behavior change because the removed method had no callers.

Validation:

  • Ruff passes for the changed file
  • 1,440 platform-applicable tests passed and 2 were skipped
  • git diff --check passed

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sromoam

sromoam commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@hugosmoreira thank you for this PR!
We're working hard on a 0.7.0 release right now, so please bear with us, but I really appreciate this and will review no later than EOW next week. But may be able to sneak it into 0.7.0

@sromoam
sromoam marked this pull request as ready for review August 6, 2026 18:51
@sromoam
sromoam self-requested a review as a code owner August 6, 2026 18:51

@sromoam sromoam 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.

Looks good, looks like we found a legit dead code path that is safe for delete.

@sromoam
sromoam merged commit b78c1f0 into awslabs:dev Aug 6, 2026
sromoam added a commit to Pybsama/stickler that referenced this pull request Aug 13, 2026
Closes awslabs#213 for everything currently mergeable. awslabs#159 and awslabs#183 have shipped
with no entry; awslabs#146 and awslabs#198 are still open, so their entries belong with
those PRs rather than here.

awslabs#159 gets three entries, two Fixed and one Changed. Verified each against the
merge parent (b78c1f0) rather than trusting the issue text, which turned out
to describe the export shape wrongly:

    to_json_schema()      before {"type": "string"}
                          after  {"type": ["string", "null"]}
    model_json_schema()   before {"type": "string"}
                          after  {"anyOf": [{"type": "string"}, {"type": "null"}]}

The issue said both emit `anyOf`. Only `model_json_schema()` does -- that is
Pydantic rendering `Optional[T]` -- while `to_json_schema()` uses the list
form. The entry documents both, since a reader feeding either into a validator
needs the one they actually call.

Also confirmed the other claims held before writing them up: explicit `null`
now accepted for an optional schema-built field (ValidationError before),
required fields still reject `None`, re-import still idempotent, and
`is_structured_field_type()` now returns True for `Optional[SomeModel]` where
it returned False. That last one needed a FieldInfo rather than a bare
annotation to reproduce -- passing the annotation returns False on both sides,
which would have read as the claim being wrong.

awslabs#183 deliberately gets no entry: 44 lines of a duplicate private helper
removed, no runtime change. The issue asked for a decision rather than a
default, so the convention is now written down in RELEASING.md -- entries are
for changes a user can observe, internal refactors are git log's job. 0.6.0's
notes already followed this; it just had not been stated.

Added a CHANGELOG checkbox to the PR template, phrased so "not needed" is an
explicit choice rather than an omission. Every gap in awslabs#213 is a PR that passed
review without one, which points at the process rather than the authors.

Suite 1746 passed, 2 skipped; mkdocs clean; the template's relative link to
RELEASING.md resolves.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove dead duplicate FieldHelper.is_structured_field_type (no union handling, no callers)

2 participants