From 97e58d7c6cda63e53c4c5fc2ce90215e14c678db Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 16:49:04 +0000 Subject: [PATCH 1/4] feat(codegen): propagate JSON Schema deprecated:true as // Deprecated: doc comment For struct fields where the source JSON Schema property declares `deprecated: true`, generate.py now emits a preceding standalone `// Deprecated: ` line rather than a trailing inline comment. The preceding-line form is required by Go's documentation comment convention (go.dev/doc/comment) for the deprecation to be recognised by gopls and go doc. Also adds adcp/schemas/test_generate.py (Python unittest) with 5 test cases covering deprecated, non-deprecated, and mixed-struct scenarios, and adcp/generator_test.go which shells out to run those Python tests under `go test ./...` so CI coverage does not require a separate workflow step. No types_gen.go changes: no current schema in the 3.0 bundle has `deprecated: true`. The generated output will update automatically when the AdCP 3.1 schema bundle lands and `download.sh` + `generate.py` are re-run (adcontextprotocol/adcp#4904). Refs #134 https://claude.ai/code/session_01TXU3btrUh956CDjk3rm8sL --- adcp/generator_test.go | 24 +++++++ adcp/schemas/generate.py | 7 +- adcp/schemas/test_generate.py | 116 ++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 adcp/generator_test.go create mode 100644 adcp/schemas/test_generate.py diff --git a/adcp/generator_test.go b/adcp/generator_test.go new file mode 100644 index 0000000..ff8ccb0 --- /dev/null +++ b/adcp/generator_test.go @@ -0,0 +1,24 @@ +package adcp_test + +import ( + "os/exec" + "testing" +) + +// TestGeneratorDeprecatedPropagation runs the Python unit tests for +// adcp/schemas/generate.py to verify that JSON Schema deprecated:true +// properties produce a preceding // Deprecated: doc comment in the emitted +// Go struct. This bridges the Python test suite into go test ./... so CI +// coverage does not require a separate workflow step. +// +// python3 is a build-time dependency of this module (generate.py and lint.py +// are already invoked by the CI test job), so exec'ing it here is safe. +func TestGeneratorDeprecatedPropagation(t *testing.T) { + cmd := exec.Command("python3", "-m", "unittest", "discover", + "-s", "schemas", "-p", "test_*.py", "-v") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("Python generator tests failed:\n%s", out) + } + t.Logf("Python generator tests:\n%s", out) +} diff --git a/adcp/schemas/generate.py b/adcp/schemas/generate.py index 511a39d..6ca2c1d 100644 --- a/adcp/schemas/generate.py +++ b/adcp/schemas/generate.py @@ -440,7 +440,12 @@ def schema_to_struct(name, schema): desc = safe_comment(prop.get('description', ''), 80) comment = f' // {desc}' if desc else '' - fields.append(f'\t{go_name} {go_type} {tag}{comment}') + if prop.get('deprecated', False): + deprecated_msg = desc if desc else 'No replacement specified.' + fields.append(f'\t// Deprecated: {deprecated_msg}') + fields.append(f'\t{go_name} {go_type} {tag}') + else: + fields.append(f'\t{go_name} {go_type} {tag}{comment}') desc = safe_comment(schema.get('description', ''), 100) doc = f'// {name} — {desc}\n' if desc else '' diff --git a/adcp/schemas/test_generate.py b/adcp/schemas/test_generate.py new file mode 100644 index 0000000..0489dae --- /dev/null +++ b/adcp/schemas/test_generate.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +"""Unit tests for generate.py — run with: python3 -m unittest test_generate""" + +import sys +import unittest +from pathlib import Path + +# Allow direct import from the same directory. +sys.path.insert(0, str(Path(__file__).parent)) +import generate as gen + + +class TestDeprecatedField(unittest.TestCase): + """Tests that deprecated: true in a JSON Schema property produces a + preceding // Deprecated: doc comment in the emitted Go struct field. + + Go's documentation comment convention requires the // Deprecated: paragraph + to immediately precede the symbol declaration (see go.dev/doc/comment). + A trailing inline comment (e.g. `Field string \`json:"f"\` // Deprecated:`) + is not recognized by gopls or go doc and will not surface the deprecation + to IDE users or go documentation consumers.""" + + def _struct_lines(self, schema): + """Return the struct body lines (inside the braces) as a list.""" + output = gen.schema_to_struct("TestStruct", schema) + lines = output.splitlines() + # Strip the type header and closing brace. + return [l for l in lines if l.startswith('\t')] + + def test_deprecated_field_emits_preceding_comment(self): + schema = { + "type": "object", + "properties": { + "status": { + "type": "string", + "deprecated": True, + "description": "Use new_status instead.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 2, + "Deprecated field should produce exactly 2 lines: comment + field") + self.assertEqual(lines[0], "\t// Deprecated: Use new_status instead.") + self.assertTrue(lines[1].startswith("\tStatus string"), + f"Field line should start with field declaration, got: {lines[1]!r}") + # The field line must NOT carry a trailing inline comment. + self.assertNotIn("//", lines[1], + "Trailing inline comment suppressed for deprecated fields") + + def test_deprecated_field_without_description_uses_fallback(self): + schema = { + "type": "object", + "properties": { + "old_field": { + "type": "string", + "deprecated": True, + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(lines[0], "\t// Deprecated: No replacement specified.") + + def test_non_deprecated_field_unchanged(self): + schema = { + "type": "object", + "properties": { + "name": { + "type": "string", + "description": "The display name.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 1, + "Non-deprecated field should produce exactly one line") + self.assertIn("// The display name.", lines[0], + "Non-deprecated field should carry trailing description comment") + + def test_non_deprecated_field_no_description(self): + schema = { + "type": "object", + "properties": { + "count": {"type": "integer"} + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 1) + self.assertNotIn("//", lines[0]) + + def test_mixed_struct_deprecated_and_normal(self): + """Regression: other fields in the same struct are unaffected.""" + schema = { + "type": "object", + "required": ["id"], + "properties": { + "id": {"type": "string", "description": "Unique identifier."}, + "status": { + "type": "string", + "deprecated": True, + "description": "Use new_status instead.", + }, + "new_status": {"type": "string", "description": "Current status."}, + }, + } + lines = self._struct_lines(schema) + # id: 1 line, status: 2 lines (comment + field), new_status: 1 line = 4 total + self.assertEqual(len(lines), 4) + deprecated_comment_idx = next( + i for i, l in enumerate(lines) if "// Deprecated:" in l + ) + self.assertIn("Status string", lines[deprecated_comment_idx + 1]) + + +if __name__ == "__main__": + unittest.main() From ed410c2a447fd876ebbdc5bb61cbea2ffa2f53a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 16:56:49 +0000 Subject: [PATCH 2/4] fixup! feat(codegen): propagate JSON Schema deprecated:true as // Deprecated: doc comment Strip redundant leading "Deprecated: " prefix from description before using it as the // Deprecated: message, preventing double-prefix output ("// Deprecated: Deprecated: ...") when schema authors set both deprecated:true and start the description with "Deprecated: ". Stripping is case-insensitive. Two new test cases cover the already-prefixed and lowercase-prefix variants. Refs #134 https://claude.ai/code/session_01TXU3btrUh956CDjk3rm8sL --- adcp/schemas/generate.py | 5 ++++- adcp/schemas/test_generate.py | 42 ++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/adcp/schemas/generate.py b/adcp/schemas/generate.py index 6ca2c1d..1654742 100644 --- a/adcp/schemas/generate.py +++ b/adcp/schemas/generate.py @@ -441,7 +441,10 @@ def schema_to_struct(name, schema): comment = f' // {desc}' if desc else '' if prop.get('deprecated', False): - deprecated_msg = desc if desc else 'No replacement specified.' + # Strip a leading "Deprecated: " some authors add to description when + # they also set deprecated:true, to avoid "// Deprecated: Deprecated: ..." + raw = re.sub(r'^deprecated:\s*', '', desc, flags=re.IGNORECASE) if desc else '' + deprecated_msg = raw if raw else 'No replacement specified.' fields.append(f'\t// Deprecated: {deprecated_msg}') fields.append(f'\t{go_name} {go_type} {tag}') else: diff --git a/adcp/schemas/test_generate.py b/adcp/schemas/test_generate.py index 0489dae..1d58f6b 100644 --- a/adcp/schemas/test_generate.py +++ b/adcp/schemas/test_generate.py @@ -107,10 +107,50 @@ def test_mixed_struct_deprecated_and_normal(self): # id: 1 line, status: 2 lines (comment + field), new_status: 1 line = 4 total self.assertEqual(len(lines), 4) deprecated_comment_idx = next( - i for i, l in enumerate(lines) if "// Deprecated:" in l + (i for i, l in enumerate(lines) if "// Deprecated:" in l), None ) + self.assertIsNotNone(deprecated_comment_idx, + "Expected a // Deprecated: line in mixed struct output") self.assertIn("Status string", lines[deprecated_comment_idx + 1]) + def test_deprecated_field_description_already_prefixed(self): + """When a schema author sets both deprecated:true and a description that + starts with 'Deprecated: ', the generator must not emit + '// Deprecated: Deprecated: ...' (double-prefix).""" + schema = { + "type": "object", + "properties": { + "axe_include_segment": { + "type": "string", + "deprecated": True, + "description": "Deprecated: Use TMP provider fields instead.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 2) + self.assertEqual( + lines[0], + "\t// Deprecated: Use TMP provider fields instead.", + "Double 'Deprecated:' prefix must be stripped", + ) + self.assertNotIn("Deprecated: Deprecated:", lines[0]) + + def test_deprecated_case_insensitive_prefix_strip(self): + """Prefix stripping is case-insensitive (e.g. 'deprecated:' lowercase).""" + schema = { + "type": "object", + "properties": { + "old_field": { + "type": "string", + "deprecated": True, + "description": "deprecated: use new_field.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(lines[0], "\t// Deprecated: use new_field.") + if __name__ == "__main__": unittest.main() From 84b51fe8bdb6f3c48408acae295a7e88c6371538 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 21 May 2026 16:59:11 +0000 Subject: [PATCH 3/4] fixup! feat(codegen): propagate JSON Schema deprecated:true as // Deprecated: doc comment Clarify generator_test.go doc comment: the adcp/ module has no CI step (pre-existing gap). Replace misleading "CI does not require a separate workflow step" with an accurate note and follow-up pointer. Refs #134 https://claude.ai/code/session_01TXU3btrUh956CDjk3rm8sL --- adcp/generator_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/adcp/generator_test.go b/adcp/generator_test.go index ff8ccb0..33ba476 100644 --- a/adcp/generator_test.go +++ b/adcp/generator_test.go @@ -8,11 +8,13 @@ import ( // TestGeneratorDeprecatedPropagation runs the Python unit tests for // adcp/schemas/generate.py to verify that JSON Schema deprecated:true // properties produce a preceding // Deprecated: doc comment in the emitted -// Go struct. This bridges the Python test suite into go test ./... so CI -// coverage does not require a separate workflow step. +// Go struct. Run with: go test ./... from the adcp/ directory. // -// python3 is a build-time dependency of this module (generate.py and lint.py -// are already invoked by the CI test job), so exec'ing it here is safe. +// Note: the adcp/ module currently has no dedicated CI step — this is a +// pre-existing gap for all adcp/ tests. Adding a "Test adcp module" step +// (working-directory: adcp) to .github/workflows/ci.yml is tracked as a +// follow-up (see issue #134). python3 is already a CI dependency (generate.py +// and lint.py are invoked in the test job), so exec'ing it here is safe. func TestGeneratorDeprecatedPropagation(t *testing.T) { cmd := exec.Command("python3", "-m", "unittest", "discover", "-s", "schemas", "-p", "test_*.py", "-v") From 5faea744211a8671ca0ab97e7110f951b2227468 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 26 May 2026 17:49:35 +0000 Subject: [PATCH 4/4] test(codegen): consolidate generator tests into generate_test.py, align discovery pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Merge TestDeprecatedField from test_generate.py into generate_test.py so all schema generator tests live in one file with a consistent *_test.py naming convention - Remove test_generate.py (now superseded) - Update generator_test.go to use *_test.py discovery pattern, matching the CI "Test schema generator" step exactly — previously test_generate.py was invisible to CI and generate_test.py (from #183) was invisible to the Go test https://claude.ai/code/session_018Gmdum6TsUTwpA6SXHjLLv --- adcp/generator_test.go | 17 ++-- adcp/schemas/generate_test.py | 114 +++++++++++++++++++++++++ adcp/schemas/test_generate.py | 156 ---------------------------------- 3 files changed, 119 insertions(+), 168 deletions(-) delete mode 100644 adcp/schemas/test_generate.py diff --git a/adcp/generator_test.go b/adcp/generator_test.go index 33ba476..09ba4f3 100644 --- a/adcp/generator_test.go +++ b/adcp/generator_test.go @@ -5,19 +5,12 @@ import ( "testing" ) -// TestGeneratorDeprecatedPropagation runs the Python unit tests for -// adcp/schemas/generate.py to verify that JSON Schema deprecated:true -// properties produce a preceding // Deprecated: doc comment in the emitted -// Go struct. Run with: go test ./... from the adcp/ directory. -// -// Note: the adcp/ module currently has no dedicated CI step — this is a -// pre-existing gap for all adcp/ tests. Adding a "Test adcp module" step -// (working-directory: adcp) to .github/workflows/ci.yml is tracked as a -// follow-up (see issue #134). python3 is already a CI dependency (generate.py -// and lint.py are invoked in the test job), so exec'ing it here is safe. -func TestGeneratorDeprecatedPropagation(t *testing.T) { +// TestGeneratorPython runs all schema generator Python unit tests. Uses the +// same *_test.py discovery pattern as the CI "Test schema generator" step so +// the two paths stay in sync. Run with: go test ./... from the adcp/ directory. +func TestGeneratorPython(t *testing.T) { cmd := exec.Command("python3", "-m", "unittest", "discover", - "-s", "schemas", "-p", "test_*.py", "-v") + "-s", "schemas", "-p", "*_test.py", "-v") out, err := cmd.CombinedOutput() if err != nil { t.Fatalf("Python generator tests failed:\n%s", out) diff --git a/adcp/schemas/generate_test.py b/adcp/schemas/generate_test.py index 311229d..d91e63f 100644 --- a/adcp/schemas/generate_test.py +++ b/adcp/schemas/generate_test.py @@ -4,6 +4,120 @@ import generate +class TestDeprecatedField(unittest.TestCase): + """Tests that deprecated: true in a JSON Schema property produces a + preceding // Deprecated: doc comment in the emitted Go struct field.""" + + def _struct_lines(self, schema): + output = generate.schema_to_struct("TestStruct", schema) + return [l for l in output.splitlines() if l.startswith('\t')] + + def test_deprecated_field_emits_preceding_comment(self): + schema = { + "type": "object", + "properties": { + "status": { + "type": "string", + "deprecated": True, + "description": "Use new_status instead.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 2, + "Deprecated field should produce exactly 2 lines: comment + field") + self.assertEqual(lines[0], "\t// Deprecated: Use new_status instead.") + self.assertTrue(lines[1].startswith("\tStatus string"), + f"Field line should start with field declaration, got: {lines[1]!r}") + self.assertNotIn("//", lines[1], + "Trailing inline comment suppressed for deprecated fields") + + def test_deprecated_field_without_description_uses_fallback(self): + schema = { + "type": "object", + "properties": { + "old_field": {"type": "string", "deprecated": True} + }, + } + lines = self._struct_lines(schema) + self.assertEqual(lines[0], "\t// Deprecated: No replacement specified.") + + def test_non_deprecated_field_unchanged(self): + schema = { + "type": "object", + "properties": { + "name": {"type": "string", "description": "The display name."} + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 1, + "Non-deprecated field should produce exactly one line") + self.assertIn("// The display name.", lines[0]) + + def test_non_deprecated_field_no_description(self): + schema = { + "type": "object", + "properties": {"count": {"type": "integer"}}, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 1) + self.assertNotIn("//", lines[0]) + + def test_mixed_struct_deprecated_and_normal(self): + """Regression: other fields in the same struct are unaffected.""" + schema = { + "type": "object", + "required": ["id"], + "properties": { + "id": {"type": "string", "description": "Unique identifier."}, + "status": { + "type": "string", + "deprecated": True, + "description": "Use new_status instead.", + }, + "new_status": {"type": "string", "description": "Current status."}, + }, + } + lines = self._struct_lines(schema) + self.assertEqual(len(lines), 4) + deprecated_comment_idx = next( + (i for i, l in enumerate(lines) if "// Deprecated:" in l), None + ) + self.assertIsNotNone(deprecated_comment_idx) + self.assertIn("Status string", lines[deprecated_comment_idx + 1]) + + def test_deprecated_field_double_prefix_stripped(self): + """Schema author sets deprecated:true and prefixes description with + 'Deprecated: ' — generator must not emit '// Deprecated: Deprecated: …'.""" + schema = { + "type": "object", + "properties": { + "axe_include_segment": { + "type": "string", + "deprecated": True, + "description": "Deprecated: Use TMP provider fields instead.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(lines[0], "\t// Deprecated: Use TMP provider fields instead.") + self.assertNotIn("Deprecated: Deprecated:", lines[0]) + + def test_deprecated_case_insensitive_prefix_strip(self): + schema = { + "type": "object", + "properties": { + "old_field": { + "type": "string", + "deprecated": True, + "description": "deprecated: use new_field.", + } + }, + } + lines = self._struct_lines(schema) + self.assertEqual(lines[0], "\t// Deprecated: use new_field.") + + def scalar_or_array_schema(min_items=1, description="Test union"): return { "description": description, diff --git a/adcp/schemas/test_generate.py b/adcp/schemas/test_generate.py deleted file mode 100644 index 1d58f6b..0000000 --- a/adcp/schemas/test_generate.py +++ /dev/null @@ -1,156 +0,0 @@ -#!/usr/bin/env python3 -"""Unit tests for generate.py — run with: python3 -m unittest test_generate""" - -import sys -import unittest -from pathlib import Path - -# Allow direct import from the same directory. -sys.path.insert(0, str(Path(__file__).parent)) -import generate as gen - - -class TestDeprecatedField(unittest.TestCase): - """Tests that deprecated: true in a JSON Schema property produces a - preceding // Deprecated: doc comment in the emitted Go struct field. - - Go's documentation comment convention requires the // Deprecated: paragraph - to immediately precede the symbol declaration (see go.dev/doc/comment). - A trailing inline comment (e.g. `Field string \`json:"f"\` // Deprecated:`) - is not recognized by gopls or go doc and will not surface the deprecation - to IDE users or go documentation consumers.""" - - def _struct_lines(self, schema): - """Return the struct body lines (inside the braces) as a list.""" - output = gen.schema_to_struct("TestStruct", schema) - lines = output.splitlines() - # Strip the type header and closing brace. - return [l for l in lines if l.startswith('\t')] - - def test_deprecated_field_emits_preceding_comment(self): - schema = { - "type": "object", - "properties": { - "status": { - "type": "string", - "deprecated": True, - "description": "Use new_status instead.", - } - }, - } - lines = self._struct_lines(schema) - self.assertEqual(len(lines), 2, - "Deprecated field should produce exactly 2 lines: comment + field") - self.assertEqual(lines[0], "\t// Deprecated: Use new_status instead.") - self.assertTrue(lines[1].startswith("\tStatus string"), - f"Field line should start with field declaration, got: {lines[1]!r}") - # The field line must NOT carry a trailing inline comment. - self.assertNotIn("//", lines[1], - "Trailing inline comment suppressed for deprecated fields") - - def test_deprecated_field_without_description_uses_fallback(self): - schema = { - "type": "object", - "properties": { - "old_field": { - "type": "string", - "deprecated": True, - } - }, - } - lines = self._struct_lines(schema) - self.assertEqual(lines[0], "\t// Deprecated: No replacement specified.") - - def test_non_deprecated_field_unchanged(self): - schema = { - "type": "object", - "properties": { - "name": { - "type": "string", - "description": "The display name.", - } - }, - } - lines = self._struct_lines(schema) - self.assertEqual(len(lines), 1, - "Non-deprecated field should produce exactly one line") - self.assertIn("// The display name.", lines[0], - "Non-deprecated field should carry trailing description comment") - - def test_non_deprecated_field_no_description(self): - schema = { - "type": "object", - "properties": { - "count": {"type": "integer"} - }, - } - lines = self._struct_lines(schema) - self.assertEqual(len(lines), 1) - self.assertNotIn("//", lines[0]) - - def test_mixed_struct_deprecated_and_normal(self): - """Regression: other fields in the same struct are unaffected.""" - schema = { - "type": "object", - "required": ["id"], - "properties": { - "id": {"type": "string", "description": "Unique identifier."}, - "status": { - "type": "string", - "deprecated": True, - "description": "Use new_status instead.", - }, - "new_status": {"type": "string", "description": "Current status."}, - }, - } - lines = self._struct_lines(schema) - # id: 1 line, status: 2 lines (comment + field), new_status: 1 line = 4 total - self.assertEqual(len(lines), 4) - deprecated_comment_idx = next( - (i for i, l in enumerate(lines) if "// Deprecated:" in l), None - ) - self.assertIsNotNone(deprecated_comment_idx, - "Expected a // Deprecated: line in mixed struct output") - self.assertIn("Status string", lines[deprecated_comment_idx + 1]) - - def test_deprecated_field_description_already_prefixed(self): - """When a schema author sets both deprecated:true and a description that - starts with 'Deprecated: ', the generator must not emit - '// Deprecated: Deprecated: ...' (double-prefix).""" - schema = { - "type": "object", - "properties": { - "axe_include_segment": { - "type": "string", - "deprecated": True, - "description": "Deprecated: Use TMP provider fields instead.", - } - }, - } - lines = self._struct_lines(schema) - self.assertEqual(len(lines), 2) - self.assertEqual( - lines[0], - "\t// Deprecated: Use TMP provider fields instead.", - "Double 'Deprecated:' prefix must be stripped", - ) - self.assertNotIn("Deprecated: Deprecated:", lines[0]) - - def test_deprecated_case_insensitive_prefix_strip(self): - """Prefix stripping is case-insensitive (e.g. 'deprecated:' lowercase).""" - schema = { - "type": "object", - "properties": { - "old_field": { - "type": "string", - "deprecated": True, - "description": "deprecated: use new_field.", - } - }, - } - lines = self._struct_lines(schema) - self.assertEqual(lines[0], "\t// Deprecated: use new_field.") - - -if __name__ == "__main__": - unittest.main()