fix: shorten generic type definition names in JSON schema

[akfaiz]

Description

Generic struct names like Response[pkg.User] were producing verbose schema definition names containing the full import path (e.g. ResponseGithubComOaswrapSpecUser). A shortenGenericName interceptor now strips the package path from type arguments, producing cleaner names like ResponseUser.

Closes #

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Refactor / code quality
• Other (describe):

Checklist

• make check passes locally (sync + tidy + lint + test)
• Tests added or updated to cover the change
• Golden files updated if spec output changed (make test-update)
• Relevant documentation updated (README, adapter README, etc.)
• Commit messages follow Conventional Commits

Notes for Reviewers

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[llamapreview]

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Request Changes

This PR introduces a breaking change by always applying a new interceptor when no config is provided, and conflicts with custom interceptors, potentially disabling the intended feature.

📄 Documentation Diagram

This diagram documents the updated JSON schema generation flow with the new generic name shortening interceptor.

sequenceDiagram
    participant User
    participant Reflector
    participant Options as getJSONSchemaOpts
    participant Interceptor as shortenGenericName
    User->>Reflector: Request schema
    Reflector->>Options: Get JSON schema options
    Options->>Options: Check config (cfg)
    alt cfg is nil or present
        Options->>Reflector: Apply interceptor
    end
    Reflector->>User: Return schema with short names
    note over Options: PR #35;57 adds shortenGenericName interceptor

Loading

🌟 Strengths

• Improves readability of JSON schema definitions by shortening verbose generic type names.

Priority	File	Category	Impact Summary	Anchors
P1	jsonschema.go	Architecture	Breaks API for users without config, changing schema output.
P1	jsonschema.go	Architecture	Silently overrides shortening with custom logic, disabling feature.
P2	jsonschema.go	Bug	Fragile parsing may corrupt schema names for complex types.
P2	jsonschema.go	Architecture	May fail on nested generics, causing malformed definition names.
P2	jsonschema.go	Maintainability	Code duplication risks future inconsistency in interceptor addition.

🔍 Notable Themes

• Configuration handling changes introduce a breaking default behavior and interceptor priority conflicts.
• Reliance on string parsing for generic types poses maintenance and correctness risks.

📈 Risk Diagram

This diagram illustrates the risk of breaking changes and interceptor conflicts in the updated schema generation.

sequenceDiagram
    participant C as Config
    participant O as getJSONSchemaOpts
    participant I1 as shortenGenericName
    participant I2 as Custom Interceptor
    C->>O: cfg is nil or not
    alt cfg is nil
        O->>O: Add shortenGenericName
        note over O: R1(P1): Breaking change for users without config
    else cfg not nil
        O->>O: Add shortenGenericName
        alt custom interceptor present
            O->>O: Override with custom
            note over O: R2(P1): Interceptor conflict disables shortening
        end
    end

Loading

⚠️ **Unanchored Suggestions (Manual Review Recommended)**

The following suggestions could not be precisely anchored to a specific line in the diff. This can happen if the code is outside the changed lines, has been significantly refactored, or if the suggestion is a general observation. Please review them carefully in the context of the full file.

---

📁 File: jsonschema.go

The shortenGenericName function's logic for splitting type arguments (strings.Split(m[2], ", ")) is fragile. It assumes the Go reflection's Type.Name() for generic types always uses a comma followed by a single space (", ") as a separator. The actual formatting is an implementation detail of the Go runtime and could vary (e.g., no trailing space after comma, or different whitespace). A mismatch would cause incorrect schema names (e.g., Response[User,Token]becomingResponseUserTokeninstead ofResponseUserToken` if the space is missing). While the updated test files pass, this reliance on an undocumented format introduces a maintenance risk.

Suggestion:

// Use a more robust separator pattern that handles optional whitespace.
args := strings.Split(m[2], ",")
for i, arg := range args {
	args[i] = strings.TrimSpace(arg)
}

Related Code:

// shortenGenericName converts "Page[some/pkg.Item]" to "PageItem".
func shortenGenericName(t reflect.Type, defaultDefName string) string {
	m := genericInstRe.FindStringSubmatch(t.Name())
	if m == nil {
		return defaultDefName
	}
	...
	args := strings.Split(m[2], ", ")
	...

---

📁 File: jsonschema.go

The regex ^(\w+)\[(.+)\]$ and the overall function logic do not account for nested generic types (e.g., Response[Page[User]]). For such types, t.Name() might produce a string like Response[Page[User]]. The regex's greedy (.+) would capture Page[User]], leading to incorrect parsing and likely a malformed schema definition name. This is a speculatve future risk, but given that generic types are a feature being actively improved, the current implementation sets a precedent that may break down with more complex use cases.

Related Code:

var genericInstRe = regexp.MustCompile(`^(\w+)\[(.+)\]$`)
...
func shortenGenericName(t reflect.Type, defaultDefName string) string {
	m := genericInstRe.FindStringSubmatch(t.Name())
	if m == nil {
		return defaultDefName
	}
	...

---

---

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

[llamapreview]

P1 | Confidence: High

The PR introduces a breaking change in behavior when cfg.ReflectorConfig is nil. Previously, if the config was nil, no JSON schema options (including interceptors) were applied. Now, the shortenGenericName interceptor is always added when cfg == nil. This changes the default output schema for all users not explicitly setting a ReflectorConfig. Since the related_context (reflector3.go and reflector31.go) shows getJSONSchemaOpts is called and its results are applied to reflector.DefaultOptions regardless of the config's presence, this unconditionally changes the public API's schema generation output for those users, a breaking change not indicated in the PR's "Breaking change" checkbox.

Suggested change

	func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []func(*jsonschema.ReflectContext) {
	var opts []func(*jsonschema.ReflectContext)
	if cfg == nil {
	opts = append(opts, jsonschema.InterceptDefName(shortenGenericName))
	return opts
	func getJSONSchemaOpts(cfg *openapi.ReflectorConfig, logger *debuglog.Logger) []func(*jsonschema.ReflectContext) {
	var opts []func(*jsonschema.ReflectContext)
	if cfg == nil {
	// Do NOT add default interceptors to preserve existing behavior when no config is provided.
	return opts
	}

[llamapreview]

P1 | Confidence: High

The PR's logic creates a conflict when a user has configured a custom InterceptDefNameFunc. The jsonschema-go library's InterceptDefName option sets a single function; the second append overrides the first. Therefore, when a custom function is provided, the new shortenGenericName feature is silently disabled. This violates the principle of least surprise and undermines the feature's stated goal of being a default improvement. Users expecting the shortening behavior alongside their custom logic will not get it, and the log message "set custom intercept definition name function" misleadingly implies addition, not replacement.

Code Suggestion:

// Define a composite interceptor that applies shortening, then custom logic if provided.
compositeInterceptor := func(t reflect.Type, defaultDefName string) string {
	shortName := shortenGenericName(t, defaultDefName)
	if cfg.InterceptDefNameFunc != nil {
		return cfg.InterceptDefNameFunc(t, shortName) // Custom func receives the shortened name.
	}
	return shortName
}
opts = append(opts, jsonschema.InterceptDefName(compositeInterceptor))
if cfg.InterceptDefNameFunc != nil {
	logger.Printf("set custom intercept definition name function")
}

[llamapreview]

P2 | Confidence: High

The shortenGenericName interceptor is added in two separate code paths (when cfg is nil and when it's not). This duplication is unnecessary and increases the risk of the two code paths diverging in the future. The logic should be consolidated. The simplest fix is to remove the early return for cfg == nil (as suggested in Finding 1 to fix the breaking change) and let the main flow handle the addition, ensuring consistency.