Drop stale PLACEHOLDER annotations that shadow upstream descriptions#5588
Open
janniklasrose wants to merge 10 commits into
Open
Drop stale PLACEHOLDER annotations that shadow upstream descriptions#5588janniklasrose wants to merge 10 commits into
janniklasrose wants to merge 10 commits into
Conversation
Drop the checked-in annotations_openapi.yml: the schema generator now derives those annotations from .codegen/cli.json in memory on every run. Merge annotations.yml and annotations_openapi_overrides.yml into a single annotations.yml whose structure mirrors the bundle configuration tree. Docs absent from the file inherit from cli.json; entries override them. The generated jsonschema.json and jsonschema_for_docs.json are unchanged. Co-authored-by: Isaac
A `description: PLACEHOLDER` marks a field that has no documentation anywhere. It is added when a field first appears undocumented, but was never removed when cli.json later gained a description for the field. Because the annotations file overrides upstream docs in the merge, the stale marker shadowed the real description, and since PLACEHOLDER is skipped when assigning annotations, the schema emitted no description at all (e.g. `vector_search_endpoints.*.target_qps`). Strip such placeholders in newAnnotationHandler before the merge: the upstream description shows through, and because the pruned map is what syncWithMissingAnnotations writes back, the annotations file sheds the stale markers in the same run. Entries that carry other hand-authored fields (e.g. deprecation_message) lose only the placeholder. Fields that are genuinely undocumented keep their TODO marker, so the result is idempotent. TestRequiredAnnotationsForNewFields now also records deletes and updates during regeneration instead of nil-panicking on them, so any future non-idempotent rewrite of annotations.yml fails with a message instead of a panic. Co-authored-by: Isaac
Co-authored-by: Isaac
PR #5574 (the annotation consolidation this branch was stacked on) landed on main, superseding the local consolidation commit. Resolved conflicts by taking main's consolidation wholesale and re-applying the PLACEHOLDER-shadowing fix, adapted to main's restructured types: annotation.File is now map[string]TypeAnnotation{Self,Fields} and Descriptor dropped Preview and gained LaunchStage/EnumLaunchStages/EnumDescriptions. Regenerated jsonschema.json, jsonschema_for_docs.json, and annotations.yml. Co-authored-by: Isaac
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
janniklasrose
commented
Jun 17, 2026
Replace the field-by-field enumeration with reflect.DeepEqual against a zero Descriptor, so adding a Descriptor field can no longer silently break the "is this entry droppable?" check after a stale placeholder is cleared. Co-authored-by: Isaac
Dropping the stale PLACEHOLDER markers restored upstream descriptions in jsonschema.json; pydabs-codegen reads that schema, so the generated Python model docstrings were stale and the validate-generated CI job failed. Regenerated via `task pydabs-codegen` (uv.lock churn reverted, as CI does). Co-authored-by: Isaac
janniklasrose
commented
Jun 17, 2026
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Collaborator
Integration test reportCommit: 00e9469
24 interesting tests: 15 SKIP, 7 KNOWN, 2 flaky
Top 26 slowest tests (at least 2 minutes):
|
pietern
approved these changes
Jun 17, 2026
…ssing-annotations Conflicts: NEXT_CHANGELOG.md
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
janniklasrose
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
description: PLACEHOLDERmarkers are now dropped when cli.json documents the field:newAnnotationHandlerstrips them before the merge, so the upstream description flows into the schema, and the sync step rewritesannotations.ymlwithout them in the same run.deprecation_message) lose only the placeholder.annotations.yml−484 lines (deletions only); both schema artifacts gain 128 description lines (23 of which replace a bare launch-stage tag).Why
A PLACEHOLDER means "no documentation anywhere". It is added when a field first appears undocumented, but nothing removed it once upstream docs arrived. The annotations file overrides upstream docs in the merge and PLACEHOLDER is skipped when assigning descriptions, so the schema ended up with no description at all — the stale marker hides the real text and then discards itself. 128 fields in the published schema were affected (e.g.
vector_search_endpoints.*.target_qps). The file cannot self-heal: a shadowed field never registers as missing, so the backlog only grows.This also fixes the bare
[Public Preview]prefixes from the launch-stage work (#5443): the prefix was there, but the description it should prefix was swallowed by this bug.Tests
TestRequiredAnnotationsForNewFieldsnow also records deletes/updates during regeneration (previously a nil-visitor panic) and asserts regeneration of the committed file is a no-op, enforcing idempotency.bundle/internal/schema+annotation+bundle/...unit tests, therefschemaacceptance test, andlintall pass; regeneration is byte-idempotent across runs.This pull request and its description were written by Isaac, an AI coding agent.