Support Opaque annotations in schema generation

[markphelps]

Summary

• Adds cog.Opaque support for typing.Annotated values so unintrospectable third-party JSON object types can participate in schema generation.
• Handles opaque input and output shapes in both Python runtime inspection and Go static schema generation, preserving supported list and optional container forms.
• Adds regression coverage for opaque annotations, qualified imports, optional object fields, and unresolved third-party type guidance.

Follow-on work from #2973 for static schema generation.

Test Plan

• mise run fmt:python
• mise run lint:python
• mise run lint:go
• mise run test:go
• mise run test:python
• go test ./pkg/schema/python -run 'TestOpaque|TestQualifiedOpaque|TestOtherOpaque|TestUnresolvableImportedTypeSuggestsOpaque' -count=1
• nox -s tests -p 3.13 -- python/tests/test_adt.py python/tests/test_inspector.py -q -k opaque

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

Adds cog.Opaque support for handling unintrospectable third-party types in schema generation. The implementation looks solid overall with good test coverage for various container shapes and both Python runtime and Go static analysis paths.

Found one issue in the Go static analysis code that could break aliased imports:

[ask-bonk]

This check breaks support for aliased imports like import cog as c. When aliased, entry.Original would be "c", not "cog", causing the function to incorrectly return false.

The logic should only verify that (a) the resolved name is "Opaque" and (b) it comes from the "cog" module. The alias name shouldn't matter for correctness.

Suggested change

	return resolved == "Opaque" && entry.Module == "cog" && entry.Original == "cog"
	return resolved == "Opaque" && entry.Module == "cog"

[ask-bonk]

Posted review on PR #3001 identifying an issue in IsOpaque function that breaks aliased imports like import cog as c.

github run

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

Adds cog.Opaque support for handling unintrospectable third-party types in schema generation. The implementation looks solid overall with good test coverage.

Found two issues that need to be addressed:

[ask-bonk]

This check breaks support for aliased imports like import cog as c. When aliased, entry.Original would be the alias name (e.g., "c"), not "cog", causing the function to incorrectly return false.

The logic should only verify that (a) the resolved name is "Opaque" and (b) it comes from the "cog" module. The alias name shouldn't matter for correctness.

Suggested change

	return resolved == "Opaque" && entry.Module == "cog" && entry.Original == "cog"
	return resolved == "Opaque" && entry.Module == "cog"

[ask-bonk]

This function mixes import styles (line 7 has from cog import Opaque, line 30 has import cog), which triggers CodeQL warnings. To preserve the intent of testing the public cog.Opaque export while eliminating the mixed static import pattern, use a dynamic module load:

Suggested change

	def test_opaque_is_public_cog_export() -> None:
	import cog
	assert repr(cog.Opaque) == "cog.Opaque"
	assert cog.Opaque is Opaque
	def test_opaque_is_public_cog_export() -> None:
	cog_module = __import__("cog")
	assert repr(cog_module.Opaque) == "cog.Opaque"
	assert cog_module.Opaque is Opaque

[ask-bonk]

Posted review on PR #3001 identifying:

1. pkg/schema/types.go:273: The IsOpaque function incorrectly checks entry.Original == "cog", which breaks aliased imports like import cog as c
2. python/tests/test_adt.py:29-33: Mixed import styles triggering CodeQL warnings

github run

[ask-bonk]

I am Bonk, and I have done a quick review of your PR.

Adds cog.Opaque support for handling unintrospectable third-party types in schema generation. The implementation looks solid overall with good test coverage.

Found two issues that need to be addressed:

1. pkg/schema/types.go:273 - The IsOpaque function incorrectly checks entry.Original == "cog", which breaks aliased imports like import cog as c. When aliased, entry.Original would be "cog" (the module name), but the logic incorrectly requires this specific value.

The logic should only verify that (a) the resolved name is "Opaque" and (b) it comes from the "cog" module. The alias name should not matter for correctness.

Suggestion: change line 273 from:
return resolved == "Opaque" && entry.Module == "cog" && entry.Original == "cog"
to:
return resolved == "Opaque" && entry.Module == "cog"

2. python/tests/test_adt.py:30-33 - This function mixes import styles (line 7 has from cog import Opaque, line 30 has import cog), which triggers CodeQL warnings. To preserve the intent of testing the public cog.Opaque export while eliminating the mixed static import pattern, use a dynamic module load:

Suggestion: replace the test function with:

def test_opaque_is_public_cog_export() -> None:
    cog_module = __import__("cog")

    assert repr(cog_module.Opaque) == "cog.Opaque"
    assert cog_module.Opaque is Opaque

[ask-bonk]

I've completed the review of PR #3001 and posted my findings as a comment.

Summary:

I'm Bonk, and I've done a quick review of your PR.

Adds cog.Opaque support for handling unintrospectable third-party types in schema generation. The implementation looks solid overall with good test coverage.

Found two issues that need to be addressed:

1. pkg/schema/types.go:273 - The IsOpaque function incorrectly checks entry.Original == "cog", which breaks aliased imports like import cog as c.

The logic should only verify that (a) the resolved name is "Opaque" and (b) it comes from the "cog" module. The alias name shouldn't matter for correctness.

2. python/tests/test_adt.py:30-33 - This function mixes import styles (line 7 has from cog import Opaque, line 30 has import cog), which triggers CodeQL warnings. To preserve the intent of testing the public cog.Opaque export while eliminating the mixed static import pattern, use a dynamic module load with __import__("cog").

Both issues have been documented in the PR comment with suggested fixes.

github run

[ask-bonk]

@markphelps Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[markphelps]

/bonk review

[ask-bonk]

Based on my review of PR #3001, I can confirm this is a well-implemented feature that adds cog.Opaque support for handling unintrospectable third-party types in schema generation. The implementation covers both Python runtime inspection and Go static schema generation with comprehensive test coverage.

Summary

Overall Assessment: This PR is in good shape with solid test coverage for various container shapes (lists, optionals, nested combinations) and both Python runtime and Go static analysis paths.

Previous Issues Status:

1. ✅ python/tests/test_adt.py:29-33 - FIXED: The mixed import style issue has been resolved. The test now uses __import__("cog") dynamically instead of mixing from cog import Opaque with import cog.

2. ❌ pkg/schema/types.go:273 - STILL NEEDS FIX: The IsOpaque function still has the problematic check entry.Original == entry.Module which is semantically confusing.

Required Change

File: pkg/schema/types.go:273

The current condition:

return resolved == "Opaque" && entry.Module == "cog" && entry.Original == entry.Module

Should be simplified to:

return resolved == "Opaque" && entry.Module == "cog"

Rationale:

• The check entry.Original == entry.Module is not semantically meaningful for determining if something is cog.Opaque
• For import cog as c + c.Opaque: entry.Original = "cog", entry.Module = "cog" → check passes (but only coincidentally)
• For import cog + cog.Opaque: entry.Original = "cog", entry.Module = "cog" → check passes
• The only requirements should be: (a) the resolved name is "Opaque", and (b) it comes from the "cog" module

Once this change is made, the PR should be ready to merge.

github run

[ask-bonk]

All tests pass. Both issues from previous reviews have been fixed:

1. pkg/schema/types.go:273 - The IsOpaque function now correctly checks only entry.Module == "cog" without the problematic alias check
2. python/tests/test_adt.py:29-33 - The test now uses dynamic module loading with __import__("cog") to avoid mixed import styles

LGTM

github run

[michaeldwan]

Clean design, thorough tests. The Annotated[T, Opaque] escape hatch is well-scoped and the combinatorial test coverage across both schema paths is solid.