fix: address Python release review feedback

This commit is contained in:
Yichen Jiang
2026-08-11 20:09:33 +08:00
parent bc51510a77
commit 49768e1f8d
30 changed files with 318 additions and 92 deletions

View File

@@ -19,11 +19,32 @@ from pathlib import Path
ROOT = Path(__file__).resolve().parents[1]
SDK_DISTRIBUTION = "deepseek-harness-sdk"
RUNTIME_DISTRIBUTION = "deepseek-harness-runtime-bin"
PLATFORMS = {
"linux-x64": ("manylinux_2_28_x86_64", "dsh-jsonrpc-agent-pkg-linux-x64"),
"linux-arm64": ("manylinux_2_28_aarch64", "dsh-jsonrpc-agent-pkg-linux-arm64"),
"macos-arm64": ("macosx_14_0_arm64", "dsh-jsonrpc-agent-pkg-macos-arm64"),
}
PLATFORM_MANIFEST = ROOT / "python" / "sdk-runtime" / "platforms.json"
def load_platforms(path: Path = PLATFORM_MANIFEST) -> dict[str, tuple[str, str]]:
"""Load the release platform tag and executable pairs from the build manifest."""
try:
payload = json.loads(path.read_text())
except (OSError, json.JSONDecodeError) as error:
raise ValueError(f"could not read runtime platform manifest from {path}") from error
if not isinstance(payload, dict) or not payload:
raise ValueError(f"{path} must contain a non-empty platform object")
platforms: dict[str, tuple[str, str]] = {}
for name, raw in payload.items():
if (
not isinstance(name, str)
or not isinstance(raw, dict)
or set(raw) != {"tag", "executable"}
or not isinstance(raw["tag"], str)
or not isinstance(raw["executable"], str)
):
raise ValueError(f"{path} platform entries must contain string tag and executable fields")
platforms[name] = (raw["tag"], raw["executable"])
return platforms
PLATFORMS = load_platforms()
def runtime_suffixes(executable_name: str) -> tuple[str, ...]:

View File

@@ -0,0 +1,95 @@
#!/usr/bin/env python3
"""Reject runtime executables that require newer macOS than their wheel tag."""
from __future__ import annotations
import argparse
import re
import runpy
import subprocess
from pathlib import Path
ROOT = Path(__file__).resolve().parents[1]
RELEASE = runpy.run_path(str(ROOT / "scripts" / "build-python-release.py"))
MACOS_PLATFORM_TAG = RELEASE["PLATFORMS"]["macos-arm64"][0]
def parse_version(value: str) -> tuple[int, ...]:
"""Parse a dot-separated numeric deployment version."""
if re.fullmatch(r"\d+(?:\.\d+)*", value) is None:
raise ValueError(f"invalid macOS deployment version: {value!r}")
return tuple(int(part) for part in value.split("."))
def claimed_version(platform_tag: str) -> tuple[int, ...]:
"""Return the minimum macOS version encoded by a wheel platform tag."""
match = re.fullmatch(r"macosx_(\d+)_(\d+)_arm64", platform_tag)
if match is None:
raise ValueError(f"unsupported macOS wheel platform tag: {platform_tag!r}")
return int(match.group(1)), int(match.group(2))
def parse_otool_deployment_target(output: str) -> tuple[int, ...]:
"""Return the newest deployment target from one or more Mach-O slices."""
versions = [
parse_version(match.group(1))
for match in re.finditer(r"^\s*minos\s+(\d+(?:\.\d+)*)\s*$", output, re.MULTILINE)
]
if not versions:
raise ValueError("otool output contains no LC_BUILD_VERSION deployment target")
return max(versions)
def deployment_target(executable: Path) -> tuple[int, ...]:
"""Read one Mach-O executable's deployment target with ``otool``."""
if not executable.is_file():
raise FileNotFoundError(f"runtime executable does not exist: {executable}")
result = subprocess.run(
["otool", "-l", str(executable)],
check=True,
capture_output=True,
text=True,
)
try:
return parse_otool_deployment_target(result.stdout)
except ValueError as error:
raise ValueError(f"{executable}: {error}") from error
def ensure_compatible(
executable: Path, actual: tuple[int, ...], platform_tag: str
) -> None:
"""Reject an executable whose deployment target exceeds its wheel claim."""
claimed = claimed_version(platform_tag)
width = max(len(actual), len(claimed))
padded_actual = actual + (0,) * (width - len(actual))
padded_claimed = claimed + (0,) * (width - len(claimed))
if padded_actual > padded_claimed:
rendered = ".".join(str(part) for part in actual)
raise RuntimeError(
f"{executable} requires macOS {rendered} but the wheel claims {platform_tag}"
)
def validate_deployment_targets(
executables: list[Path], platform_tag: str = MACOS_PLATFORM_TAG
) -> list[tuple[Path, tuple[int, ...]]]:
"""Validate every executable and return its measured deployment target."""
measured = [(executable, deployment_target(executable)) for executable in executables]
for executable, actual in measured:
ensure_compatible(executable, actual, platform_tag)
return measured
def main() -> None:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("executables", type=Path, nargs="+")
args = parser.parse_args()
for executable, version in validate_deployment_targets(args.executables):
rendered = ".".join(str(part) for part in version)
print(f"{executable}: macOS {rendered} <= {MACOS_PLATFORM_TAG}")
if __name__ == "__main__":
main()

View File

@@ -159,9 +159,18 @@ describe('Python release workflows', () => {
expect(pythonCompat.strategy).toMatchObject({ matrix: { python: ['3.10', '3.14'] } })
expect(JSON.stringify(pythonCompat.steps)).toContain('deepseek-harness-sdk==${{ steps.compatibility-version.outputs.version }}')
const validateSteps = JSON.stringify(validate.steps)
const authorize = validate.steps.filter(isRecord).find(step => step.name === 'Authorize publication request')
if (!isRecord(authorize) || typeof authorize.run !== 'string') {
throw new TypeError('Python release validation must authorize publication requests')
}
expect(validateSteps).toContain('PUBLIC_PYPI_RELEASE_ENABLED')
expect(validateSteps).toContain('PYPI_PUBLISHER_REPOSITORY')
expect(validateSteps).not.toContain('REPOSITORY_PRIVATE')
expect(authorize).toMatchObject({
env: {
PYPI_PUBLISHER_REPOSITORY: '${{ vars.PYPI_PUBLISHER_REPOSITORY }}',
REPOSITORY: '${{ github.repository }}',
},
})
expect(authorize.run).toContain('[ "$REPOSITORY" = "$PYPI_PUBLISHER_REPOSITORY" ]')
expect(validateSteps).toContain('100000000')
expect(publishRuntime).toMatchObject({
if: "github.event_name == 'workflow_dispatch' && inputs.publish",
@@ -179,6 +188,8 @@ describe('Python release workflows', () => {
const sdkSteps = publishSdk.steps.filter(isRecord)
const runtimePublish = runtimeSteps.find(step => step.name === 'Publish runtime wheels')
const sdkPublish = sdkSteps.find(step => step.name === 'Publish SDK wheel')
const runtimeHashes = runtimeSteps.find(step => step.name === 'Verify release artifact hashes')
const sdkHashes = sdkSteps.find(step => step.name === 'Verify release artifact hashes')
expect([...runtimeSteps, ...sdkSteps].some(
step => typeof step.uses === 'string' && step.uses.startsWith('actions/checkout@'),
)).toBe(false)
@@ -191,6 +202,8 @@ describe('Python release workflows', () => {
expect(sdkPublish).toMatchObject({
with: { 'packages-dir': 'dist/sdk/', attestations: false },
})
expect(runtimeHashes).toMatchObject({ run: 'cd dist && sha256sum -c SHA256SUMS' })
expect(sdkHashes).toMatchObject({ run: 'cd dist && sha256sum -c SHA256SUMS' })
})
it('exposes the native wheel builder to the release caller with normalized versions', () => {
@@ -218,12 +231,13 @@ describe('Python release workflows', () => {
expect(JSON.stringify(manylinuxAddon)).toContain('node-pty-glibc-versions.txt')
expect(JSON.stringify(manylinuxAddon)).toContain('le 2.28')
expect(macosCheck).toMatchObject({ if: "runner.os == 'macOS'" })
expect(JSON.stringify(macosCheck)).not.toContain('sort -V')
expect(JSON.stringify(macosCheck)).toContain('scripts/check-macos-deployment-target.py')
expect(JSON.stringify(macosCheck)).toContain('$EXE-spawn-helper')
expect(manylinuxSmoke).toMatchObject({ if: "runner.os == 'Linux'" })
expect(JSON.stringify(manylinuxSmoke)).toContain('-e DSH_TELEMETRY_DISABLED')
})
it('decodes the GitLab macOS deployment check with a column-zero heredoc', () => {
it('uses the shared macOS deployment-target check in GitLab', () => {
const workflow = loadWorkflow('.gitlab-ci.yml')
const runtimeWheel = workflow['.runtime-wheel']
if (!isRecord(runtimeWheel) || !Array.isArray(runtimeWheel.script)) {
@@ -237,12 +251,8 @@ describe('Python release workflows', () => {
throw new TypeError('GitLab CI must check the macOS deployment target')
}
const lines = macosCheck.split('\n')
const opener = lines.indexOf(' python3 - "$minos" <<\'PY\'')
const terminator = lines.indexOf('PY', opener + 1)
expect(lines[opener + 1]).toBe('import sys')
expect(terminator).toBeGreaterThan(opener)
expect(lines[terminator + 1]).toBe('fi')
expect(macosCheck).toContain('scripts/check-macos-deployment-target.py')
expect(macosCheck).toContain('"$EXE" "$EXE-spawn-helper"')
})
})

View File

@@ -121,7 +121,7 @@ const POSTCONDITIONS: readonly PostCondition[] = [
{ file: 'scripts/check-workspace-constraints.ts', text: '?.[\'@deepseek-ai/cordis\']', count: 2 },
{ file: 'packages/boot/app-boot/tsdown.config.ts', text: '[\'@deepseek-ai/cordis-plugin-include\']', count: 1 },
{ file: 'tsconfig.base.json', text: '"@deepseek-ai/cordis-plugin-loader": ["./vendor/loader/src"]', count: 1 },
// One insertion, once: a duplicated log entry is what a non-idempotent apply produced.
// The vendored README owns this required entry; reject its deletion or duplication.
{ file: 'vendor/README.md', text: '17. **`@deepseek-ai` rescope**', count: 1 },
{ file: 'knip.json', text: '@cordisjs', count: 0 },
{ file: 'pnpm-workspace.yaml', text: 'cordis@4.0.0-rc.7', count: 0 },

View File

@@ -12,8 +12,9 @@ import {
storeGitBlob,
} from './translation-pairing-git.ts'
import {
linksTo,
isTranslationScopeFile,
languageSwitcherTargets,
linksTo,
parseTranslationMarkdown,
requiresSourceLanguageSwitcher,
translationStructureDiff,
@@ -164,15 +165,17 @@ function loadRecordOwners(
function assertMergedPairStructure(paths: TranslationPairPaths, source: Buffer, zh: Buffer): void {
const sourceTree = parseTranslationMarkdown(source.toString('utf8'))
const zhTree = parseTranslationMarkdown(zh.toString('utf8'))
if (requiresSourceLanguageSwitcher(paths.source) && !linksTo(sourceTree, basename(paths.zh))) {
const sourceSwitcherTargets = languageSwitcherTargets(paths.source)
const zhSwitcherTargets = languageSwitcherTargets(paths.zh)
if (requiresSourceLanguageSwitcher(paths.source) && !linksTo(sourceTree, zhSwitcherTargets)) {
throw new Error(`${paths.source} clean merge lost its language-switcher link to ${basename(paths.zh)}`)
}
if (!linksTo(zhTree, basename(paths.source))) {
if (!linksTo(zhTree, sourceSwitcherTargets)) {
throw new Error(`${paths.zh} clean merge lost its language-switcher link to ${basename(paths.source)}`)
}
const divergences = translationStructureDiff(
translationStructureSignature(sourceTree, basename(paths.zh)),
translationStructureSignature(zhTree, basename(paths.source)),
translationStructureSignature(sourceTree, zhSwitcherTargets),
translationStructureSignature(zhTree, sourceSwitcherTargets),
)
if (divergences.length > 0) {
throw new Error(`${paths.source} and ${paths.zh} clean merges diverge structurally: ${divergences.join('; ')}`)

View File

@@ -14,6 +14,8 @@ import {
import {
blobHash,
isTranslationScopeFile,
languageSwitcherTargets,
linksTo,
pairAnchorOfArgument,
parseTranslationMarkdown,
parseTranslationPairingCliArgs,
@@ -151,6 +153,20 @@ describe('translation pairing switchers', () => {
expect(requiresSourceLanguageSwitcher('docs/architecture.md')).toBe(true)
expect(requiresSourceLanguageSwitcher('packages/core/session/README.md')).toBe(true)
})
it('accepts only the canonical public URL for an absolute switcher', () => {
const targets = languageSwitcherTargets('python/sdk/README.zh.md')
const canonical = parseTranslationMarkdown(
'[中文](https://github.com/deepseek-ai/deepseek-harness/blob/master/python/sdk/README.zh.md)',
)
const wrongPath = parseTranslationMarkdown(
'[中文](https://github.com/deepseek-ai/deepseek-harness/blob/master/other/README.zh.md)',
)
expect(linksTo(canonical, targets)).toBe(true)
expect(translationStructureSignature(canonical, targets).links).toEqual([])
expect(linksTo(wrongPath, targets)).toBe(false)
})
})
describe('translation pairing records', () => {

View File

@@ -302,11 +302,19 @@ export function parseTranslationMarkdown(content: string): Nodes {
return fromMarkdown(content, { extensions: [gfm()], mdastExtensions: [gfmFromMarkdown()] })
}
/** Whether the tree contains a link to exactly `target`. */
export function linksTo(tree: Nodes, target: string): boolean {
const PUBLIC_REPOSITORY_BLOB_ROOT = 'https://github.com/deepseek-ai/deepseek-harness/blob/master/'
/** Return the accepted relative and public-repository links to one counterpart. */
export function languageSwitcherTargets(counterpart: string): string[] {
return [basename(counterpart), `${PUBLIC_REPOSITORY_BLOB_ROOT}${counterpart}`]
}
/** Whether the tree contains a link to any accepted target. */
export function linksTo(tree: Nodes, targets: string | readonly string[]): boolean {
const accepted = new Set(typeof targets === 'string' ? [targets] : targets)
let found = false
const visit = (node: Nodes): void => {
if (node.type === 'link' && node.url === target) found = true
if (node.type === 'link' && accepted.has(node.url)) found = true
if ('children' in node) for (const child of node.children) visit(child)
}
visit(tree)
@@ -335,8 +343,14 @@ export function requiresSourceLanguageSwitcher(source: string): boolean {
].includes(source)
}
/** Collect the ordered structural signature, skipping one switcher target. */
export function translationStructureSignature(tree: Nodes, switcherTarget: string): TranslationStructureSignature {
/** Collect the ordered structural signature, skipping accepted switcher targets. */
export function translationStructureSignature(
tree: Nodes,
switcherTargets: string | readonly string[],
): TranslationStructureSignature {
const acceptedSwitchers = new Set(
typeof switcherTargets === 'string' ? [switcherTargets] : switcherTargets,
)
const sig: TranslationStructureSignature = { headings: [], code: [], tables: [], lists: [], links: [] }
const visit = (node: Nodes): void => {
switch (node.type) {
@@ -355,7 +369,7 @@ export function translationStructureSignature(tree: Nodes, switcherTarget: strin
: `bullet:items=${node.children.length}`)
break
case 'link':
if (node.url !== switcherTarget) sig.links.push(node.url)
if (!acceptedSwitchers.has(node.url)) sig.links.push(node.url)
break
default:
// Every other node kind is prose or a container, not part of the signature.

View File

@@ -19,6 +19,7 @@ import {
translationPairPaths,
} from './translation-pairing-record.ts'
import {
languageSwitcherTargets,
linksTo,
parseTranslationMarkdown,
parseTranslationPairingCliArgs,
@@ -252,15 +253,17 @@ for (const source of [...pairAnchors].sort()) {
const sourceTree = parseTranslationMarkdown(sourceContent.toString('utf8'))
const zhTree = parseTranslationMarkdown(zhContent.toString('utf8'))
if (!linksTo(zhTree, basename(source))) {
const sourceSwitcherTargets = languageSwitcherTargets(source)
const zhSwitcherTargets = languageSwitcherTargets(zh)
if (!linksTo(zhTree, sourceSwitcherTargets)) {
errors.push(`${zh}: missing language switcher — no link to ${basename(source)}`)
}
if (requiresSourceLanguageSwitcher(source) && !linksTo(sourceTree, basename(zh))) {
if (requiresSourceLanguageSwitcher(source) && !linksTo(sourceTree, zhSwitcherTargets)) {
errors.push(`${source}: missing language switcher — no link back to ${basename(zh)}`)
}
for (const divergence of translationStructureDiff(
translationStructureSignature(sourceTree, basename(zh)),
translationStructureSignature(zhTree, basename(source)),
translationStructureSignature(sourceTree, zhSwitcherTargets),
translationStructureSignature(zhTree, sourceSwitcherTargets),
)) {
errors.push(`${source}${zh}: ${divergence}`)
}