feat(tools): accept Unicode Python identifiers in the Python SDK renderer
The identifier test was ASCII-only, so an object with a `路径` field degraded to dict[str, Any] -- dropping every sibling field's name, requiredness and type, with no native schema behind it in Code Mode to carry them. Python identifiers are `xid_start xid_continue*`, so match that instead, and widen camelCase's split and head check to the same sets (naming `_` explicitly in the split, since it is XID_Continue). NFKC stability is a second and separate condition. CPython normalizes identifiers at compile time while a JSON key is compared as written, so a U+FB01 ligature key would be declared and reachable under its ASCII expansion, a key the tool never accepts, and two keys that normalize together would collapse into one declaration. Those names take the subscript path. Generated class names are normalized instead of rejected -- they are never matched against a key. Astral characters can now reach the class-name cap, whose slice counts UTF-16 code units, so drop a split surrogate half. Also fix two comment claims. The note said one projection reads the runtime twice per tool; the language-aware getters are installed on run_code's own definition, so it is twice, both for that schema. And the 182-bracket site's reachability is an array reached from the root through oneOf arms alone -- a union spine of any depth, not just one root union; an object ancestor restarts the chain at the 181 site.
This commit is contained in:
@@ -17,8 +17,34 @@ import { assertSupportedJsonSchema } from './json-schema.ts'
|
||||
import type { JsonSchemaNode, JsonSchemaScalar } from './json-schema.ts'
|
||||
import type { ToolSdkSchema } from './ts-types.ts'
|
||||
|
||||
/** Property names that are valid bare Python identifiers; anything else is subscripted. */
|
||||
const IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/
|
||||
/** The reference grammar's `xid_start xid_continue*`, the same set `str.isidentifier()` accepts. */
|
||||
const IDENTIFIER = /^[\p{XID_Start}_]\p{XID_Continue}*$/u
|
||||
|
||||
/**
|
||||
* Whether a name can be emitted as a bare Python identifier rather than
|
||||
* routed to the subscript/`dict[str, Any]` path.
|
||||
*
|
||||
* Python identifiers are not ASCII: `路径` is as legal a field name as `path`,
|
||||
* and rejecting it would degrade the whole enclosing object, dropping every
|
||||
* field's name, requiredness, and type — and in Code Mode the native schemas
|
||||
* are omitted, so this text is the model's only source for them.
|
||||
*
|
||||
* NFKC stability is a second and separate condition, because CPython
|
||||
* normalizes identifiers at compile time while JSON keys are compared as
|
||||
* written: `field` would be declared and reachable as `field`, so the SDK would
|
||||
* advertise a key under a spelling the harness never accepts, and two keys
|
||||
* that normalize together would collapse into one declaration. Those names
|
||||
* take the subscript path, which carries their exact bytes.
|
||||
*
|
||||
* The `ts-types` sibling keeps its own ASCII rule rather than sharing this
|
||||
* one: ECMAScript identifiers are a different set (`$`, ZWJ/ZWNJ) and are
|
||||
* never normalized, so one predicate cannot be correct for both.
|
||||
* @param name - the raw schema field or tool name.
|
||||
* @returns whether the name can be emitted bare.
|
||||
*/
|
||||
function isBareIdentifier(name: string): boolean {
|
||||
return IDENTIFIER.test(name) && name.normalize('NFKC') === name
|
||||
}
|
||||
|
||||
/**
|
||||
* Python hard keywords: reserved everywhere, so a tool or field named
|
||||
@@ -32,8 +58,7 @@ const IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/
|
||||
* one syntactic position — a statement head (``match``, ``type``), a ``match``
|
||||
* statement's clause head (``case``), or a pattern (``_``) — so ``match: str``
|
||||
* as a field and ``async def match(...)`` as a method are both legal, and
|
||||
* including
|
||||
* them would needlessly degrade common search/regex tool fields to
|
||||
* including them would needlessly degrade common search/regex tool fields to
|
||||
* ``dict[str, Any]``. Underscore-leading names are handled separately, not
|
||||
* here: a non-dunder ``__token`` name-mangles, a dunder present on
|
||||
* ``object``/``type`` resolves before the proxy hook, and implicit
|
||||
@@ -156,14 +181,26 @@ function docLines(description: unknown, indent: number): string[] {
|
||||
return [`${pad(indent)}"""${escaped}"""`]
|
||||
}
|
||||
|
||||
/** CamelCase a name into a Python type identifier (non-identifier chars split words; a non-letter head is prefixed). */
|
||||
/**
|
||||
* CamelCase a name into a Python type identifier: non-identifier characters
|
||||
* split words, `_` splits too (it is `XID_Continue`, so the split set names it
|
||||
* explicitly), and a head that cannot start an identifier takes a `Tool`
|
||||
* prefix. Unicode survives, so a `路径` field yields `路径`-based class names
|
||||
* instead of collapsing to the bare prefix. The result is NFKC-normalized:
|
||||
* these names are generated, never matched against a JSON key, so normalizing
|
||||
* is free here and keeps what CPython compiles identical to what is emitted —
|
||||
* unlike {@link isBareIdentifier}, which must reject unstable names outright.
|
||||
* @param raw - the schema field or tool name to derive from.
|
||||
* @returns a class-name segment safe to emit.
|
||||
*/
|
||||
function camelCase(raw: string): string {
|
||||
const joined = raw
|
||||
.split(/[^A-Za-z0-9]+/)
|
||||
.split(/[^\p{XID_Continue}]+|_+/u)
|
||||
.filter(part => part.length > 0)
|
||||
.map(part => `${part.charAt(0).toUpperCase()}${part.slice(1)}`)
|
||||
.join('')
|
||||
return /^[A-Za-z]/.test(joined) ? joined : `Tool${joined}`
|
||||
.normalize('NFKC')
|
||||
return /^\p{XID_Start}/u.test(joined) ? joined : `Tool${joined}`
|
||||
}
|
||||
|
||||
/** Class-name base cap keeping each emitted name — and total text — linear in schema depth. */
|
||||
@@ -191,9 +228,11 @@ const MAX_CLASS_NAME_BASE = 120
|
||||
* - Argument annotation, `async def f(self, args: chain) -> Y:` — the `(` IS
|
||||
* still open around it: 180 `list[` plus `Literal[` plus the paren, 182, the
|
||||
* worst case. Reachable only through a raw `register()` whose `parameters`
|
||||
* root opens an array chain — rooted at the array, or at an array branch of
|
||||
* a root `oneOf`, which inherits the enclosing depth because a union adds no
|
||||
* brackets. `defineTool` compiles an object root, so the annotation is a
|
||||
* is an array reached from the root through `oneOf` arms alone — the root
|
||||
* array itself, or one nested under any depth of unions, since an arm
|
||||
* inherits the enclosing depth unchanged (`A | B` opens no bracket). An
|
||||
* object ancestor takes it out of this case: its fields restart the chain at
|
||||
* the 181 site. `defineTool` compiles an object root, so the annotation is a
|
||||
* bare TypedDict class name or a one-bracket `dict[str, Any]` when that
|
||||
* object degrades — never a chain.
|
||||
*
|
||||
@@ -208,9 +247,17 @@ const MAX_CLASS_NAME_BASE = 120
|
||||
*/
|
||||
const MAX_LIST_NESTING = 180
|
||||
|
||||
/** Cap a class-name base at {@link MAX_CLASS_NAME_BASE} (see the callers for why capping keeps the render linear). */
|
||||
/**
|
||||
* Cap a class-name base at {@link MAX_CLASS_NAME_BASE} (see the callers for
|
||||
* why capping keeps the render linear). `slice` counts UTF-16 code units, so
|
||||
* an astral character straddling the boundary would be cut in half and leave a
|
||||
* lone surrogate — not an identifier character, and not even well-formed text;
|
||||
* drop it rather than emit it.
|
||||
*/
|
||||
function capClassNameBase(base: string): string {
|
||||
return base.length > MAX_CLASS_NAME_BASE ? base.slice(0, MAX_CLASS_NAME_BASE) : base
|
||||
if (base.length <= MAX_CLASS_NAME_BASE) return base
|
||||
const capped = base.slice(0, MAX_CLASS_NAME_BASE)
|
||||
return /[\uD800-\uDBFF]$/.test(capped) ? capped.slice(0, -1) : capped
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -520,7 +567,7 @@ function renderType(schema: unknown, className: string, state: RenderState): str
|
||||
// NAME-MANGLED inside class syntax (`_ClassName__token`), describing a
|
||||
// different JSON key than the registered schema — degrade like any
|
||||
// other inexpressible field name.
|
||||
if (className === '' || !entries.every(([name]) => IDENTIFIER.test(name) && !RESERVED.has(name) && !(name.startsWith('__') && !name.endsWith('__')))) {
|
||||
if (className === '' || !entries.every(([name]) => isBareIdentifier(name) && !RESERVED.has(name) && !(name.startsWith('__') && !name.endsWith('__')))) {
|
||||
state.typing.add('Any')
|
||||
finish('dict[str, Any]')
|
||||
break
|
||||
@@ -623,7 +670,7 @@ export function renderToolsSdkPy(schemas: ToolSdkSchema[]): string {
|
||||
for (const schema of sorted) {
|
||||
const argType = renderType(schema.parameters, `${camelCase(schema.name)}Args`, state)
|
||||
const outputType = renderType(schema.output, `${camelCase(schema.name)}Output`, state)
|
||||
if (IDENTIFIER.test(schema.name) && !RESERVED.has(schema.name) && !schema.name.startsWith('_')) {
|
||||
if (isBareIdentifier(schema.name) && !RESERVED.has(schema.name) && !schema.name.startsWith('_')) {
|
||||
// A docstring only documents its method when it is the FIRST statement
|
||||
// of that method's body. Emitted before the `async def` it would instead
|
||||
// become the `Tools` class docstring (for the first tool) or a dead
|
||||
|
||||
@@ -398,6 +398,106 @@ describe('renderToolsSdkPy', () => {
|
||||
expect(text).not.toContain('dict[str, Any]')
|
||||
})
|
||||
|
||||
it('keeps a non-ASCII field name as a TypedDict field and derives its class name from it', () => {
|
||||
// `路径` satisfies `xid_start xid_continue*`, so CPython accepts it as an
|
||||
// attribute and as the `TypedDict` key. Rejecting it would degrade the
|
||||
// whole object, dropping every SIBLING field's name, requiredness and type
|
||||
// too — and Code Mode omits the native schemas, so nothing else carries
|
||||
// them. The nested class name is derived from the field, so `camelCase`
|
||||
// has to pass the same characters through instead of splitting on them.
|
||||
const tool: ToolSdkSchema = {
|
||||
name: '搜索',
|
||||
description: 'Unicode identifiers.',
|
||||
parameters: {
|
||||
type: 'object',
|
||||
additionalProperties: false,
|
||||
properties: {
|
||||
路径: { type: 'string' },
|
||||
opts: { type: 'object', additionalProperties: false, properties: { 深度: { type: 'number' } } },
|
||||
},
|
||||
required: ['路径'],
|
||||
},
|
||||
output: { type: 'string' },
|
||||
}
|
||||
const text = renderToolsSdkPy([tool])
|
||||
expect(text).toContain('async def 搜索(self, args: 搜索Args) -> str:')
|
||||
expect(text).toContain('class 搜索Args(TypedDict):')
|
||||
expect(text).toContain(' 路径: str')
|
||||
expect(text).toContain('class 搜索ArgsOpts(TypedDict):')
|
||||
expect(text).toContain(' 深度: NotRequired[float]')
|
||||
expect(text).not.toContain('dict[str, Any]')
|
||||
})
|
||||
|
||||
it('degrades a field name that NFKC-normalizes to something else, which would be declared under another spelling', () => {
|
||||
// U+FB01 LATIN SMALL LIGATURE FI passes the identifier grammar, but CPython
|
||||
// normalizes identifiers at compile time while the harness compares the
|
||||
// JSON key as written: `field: str` would declare and be reachable as
|
||||
// `field`, a key the tool never accepts. Two keys that normalize together
|
||||
// would additionally collapse into one declaration. The subscript path
|
||||
// carries the exact bytes instead.
|
||||
const text = renderToolsSdkPy([
|
||||
{
|
||||
name: 'ligature',
|
||||
description: 'Normalizing field name.',
|
||||
parameters: { type: 'object', additionalProperties: false, properties: { field: { type: 'string' } } },
|
||||
output: { type: 'string' },
|
||||
},
|
||||
])
|
||||
expect(text).toContain('async def ligature(self, args: dict[str, Any]) -> str:')
|
||||
expect(text).not.toContain('field:')
|
||||
expect(text).not.toContain('field:')
|
||||
})
|
||||
|
||||
it('subscripts a tool name that NFKC-normalizes to something else, while declaring a plain Unicode one', () => {
|
||||
// Same split at the tool-name site: `路径` becomes an `async def`, the
|
||||
// ligature name cannot, because `async def find` would define `find`. The
|
||||
// subscript comment quotes the name, so its exact bytes survive, and its
|
||||
// TypedDict is still named and referenced — the name is only unusable as a
|
||||
// method, not as a class-name source (`camelCase` normalizes what it
|
||||
// derives, since a generated name is never matched against a JSON key).
|
||||
const of = (name: string): ToolSdkSchema => ({
|
||||
name,
|
||||
description: `Tool ${name}.`,
|
||||
parameters: { type: 'object', additionalProperties: false, properties: { q: { type: 'string' } }, required: ['q'] },
|
||||
output: { type: 'string' },
|
||||
})
|
||||
const text = renderToolsSdkPy([of('路径'), of('find')])
|
||||
expect(text).toContain('async def 路径(self, args: 路径Args) -> str:')
|
||||
expect(text).toContain('# tools["find"](args: FIndArgs) -> str')
|
||||
expect(text).toContain('class FIndArgs(TypedDict):')
|
||||
expect(text).not.toContain('async def find')
|
||||
expect(text).not.toContain('async def find')
|
||||
})
|
||||
|
||||
it('drops a surrogate half rather than cutting a pair when capping an astral class-name base', () => {
|
||||
// Class-name bases are capped by `slice`, which counts UTF-16 code units,
|
||||
// so a boundary landing inside an astral pair would leave a lone high
|
||||
// surrogate — not an identifier character, and not encodable text. Padding
|
||||
// with one ASCII character shifts the boundary onto the pair.
|
||||
// U+10330 GOTHIC LETTER AHSA: XID_Start and NFKC-stable, unlike `𝕏`, which
|
||||
// NFKC-folds to ASCII `X` and so never reaches the boundary at all.
|
||||
const AHSA = String.fromCodePoint(0x10330)
|
||||
const className = (pad: string): string => {
|
||||
const text = renderToolsSdkPy([
|
||||
{
|
||||
name: `${pad}${AHSA.repeat(200)}`,
|
||||
description: 'Astral name.',
|
||||
parameters: { type: 'object', additionalProperties: false, properties: { a: { type: 'string' } } },
|
||||
output: { type: 'string' },
|
||||
},
|
||||
])
|
||||
// The base is `${camelCase(name)}Args` capped to 120 code units, so the
|
||||
// `Args` suffix itself is cut off here; match the declaration instead.
|
||||
return /^class (.+)\(TypedDict\):$/mu.exec(text)![1]!
|
||||
}
|
||||
// Each character is 2 code units, so an unpadded name fills the cap with 60
|
||||
// whole characters; one ASCII character of padding puts the boundary inside
|
||||
// the 60th pair, and that half is dropped rather than emitted.
|
||||
expect(className('')).toBe(AHSA.repeat(60))
|
||||
expect(className('x')).toBe(`X${AHSA.repeat(59)}`)
|
||||
expect(className('x')).toHaveLength(119)
|
||||
})
|
||||
|
||||
it('declares a closed empty object with omitted properties as an empty TypedDict, not dict[str, Any]', () => {
|
||||
// `{ type: 'object', additionalProperties: false }` with no `properties`
|
||||
// is a closed empty object — no key accepted — exactly as the validator
|
||||
@@ -580,8 +680,10 @@ describe('renderToolsSdkPy', () => {
|
||||
// The worst of the three emission sites: the parameter list's `(` is still
|
||||
// open around this annotation, so 180 `list[` plus the innermost bracket
|
||||
// plus that paren is 182 of CPython's 200. Only a raw `register()` whose
|
||||
// `parameters` root opens an array chain reaches it — rooted at the array,
|
||||
// or at an array branch of a root `oneOf`, since a union adds no brackets.
|
||||
// `parameters` is an array reached from the root through `oneOf` arms
|
||||
// alone gets there — the root array itself, or one under any depth of
|
||||
// unions, since an arm inherits the enclosing depth unchanged. An object
|
||||
// ancestor takes it out of this case: its fields restart at the 181 site.
|
||||
// `defineTool` compiles an object root, whose annotation is a bare
|
||||
// TypedDict name or a one-bracket `dict[str, Any]`, never a chain.
|
||||
const rooted = (depth: number): ToolSdkSchema => {
|
||||
@@ -602,13 +704,25 @@ describe('renderToolsSdkPy', () => {
|
||||
// rather than on another `list[`, so the count cannot grow past that.
|
||||
expect(renderToolsSdkPy([rooted(181)]))
|
||||
.toContain(`async def rooted(self, args: ${'list['.repeat(180)}Any${']'.repeat(180)}) -> str:`)
|
||||
// A root union reaches the same 182: its branches inherit the enclosing
|
||||
// depth because `A | B` opens nothing, so the chain under one of them
|
||||
// starts at 0 exactly as the array-rooted case does.
|
||||
const union = { ...rooted(180), parameters: { oneOf: [rooted(180).parameters, { type: 'string' }] } }
|
||||
const text = renderToolsSdkPy([union])
|
||||
expect(text).toContain(`args: ${'list['.repeat(180)}Literal["x"]${']'.repeat(180)} | str) -> str:`)
|
||||
// A union spine reaches the same 182, at any number of arms deep: each arm
|
||||
// inherits the enclosing depth because `A | B` opens nothing, so the chain
|
||||
// under the innermost one still starts at 0. Three unions here, to pin that
|
||||
// it is the whole `oneOf`-only path and not just a single root union.
|
||||
let spine: Record<string, unknown> = rooted(180).parameters
|
||||
for (let i = 0; i < 3; i++) spine = { oneOf: [spine, { type: 'string' }] }
|
||||
const text = renderToolsSdkPy([{ ...rooted(180), parameters: spine }])
|
||||
const chain = `${'list['.repeat(180)}Literal["x"]${']'.repeat(180)}`
|
||||
expect(text).toContain(`args: ${chain} | str | str | str) -> str:`)
|
||||
expect(text.split('async def rooted(self, args: ')[1]!.split(') -> str:')[0]!.split('[').length - 1).toBe(181)
|
||||
// An object ancestor is the boundary of that path: the field it declares is
|
||||
// a class-body line, so the same chain lands on the 181 site instead.
|
||||
const boxed = renderToolsSdkPy([
|
||||
{
|
||||
...rooted(180),
|
||||
parameters: { type: 'object', properties: { rows: rooted(180).parameters }, required: ['rows'] },
|
||||
},
|
||||
])
|
||||
expect(boxed).toContain(` rows: ${'list['.repeat(179)}Any${']'.repeat(179)}`)
|
||||
})
|
||||
|
||||
it('renders a deeply nested oneOf chain in linear time (no per-level re-materialization)', () => {
|
||||
|
||||
Reference in New Issue
Block a user