Skip to content

Stop the TypeScript reader coercing ONIX values to numbers - #64

Open
kogai wants to merge 2 commits into
claude/fix-package-metadatafrom
claude/ts-reader-no-value-coercion
Open

kogai wants to merge 2 commits into
claude/fix-package-metadatafrom
claude/ts-reader-no-value-coercion

Conversation

@kogai

@kogai kogai commented Sep 13, 2026

Copy link
Copy Markdown
Owner

生成される TypeScript クライアントが ONIX のデータを壊しています#59 のレビューで発覚し、ADR-0005 に既知の問題として記録していた件の対応です。

base は #62(npm 系のチェーン #56 → #57 → #59 → #62 → 本 PR)。

何が壊れているか

fast-xml-parser は既定で、数値に見える値をその型に変換します。実測値です。

既定                  {"NotificationType":1,"ProductIDType":2,"IDValue":62124983,
                       "ISBN13":9784062124983,"Price":1200.5}

parseTagValue:false   {"NotificationType":"01","ProductIDType":"02","IDValue":"062124983",
                       "ISBN13":"9784062124983","Price":"1200.50"}
  • "01"1。ONIX のコード値は先頭ゼロを含めて意味を持つ 2 桁の文字列で、1 は別物です
  • "062124983"62124983。識別子から先頭ゼロが落ちます
  • "1200.50"1200.5金額の桁が落ちます

これは v5 で入った挙動ではありません。v3 でも同じで、最初から壊れていました(#59 のレビューで、両バージョンが同一に壊すことまで確認されています)。

生成された型が嘘をついていた

generated/typescript/v2/code.ts はコード型を文字列として宣言しています。

export type NotificationType = string

宣言は string、実際に返るのは number でした。

さらに、Go 側は同じスキーマから一貫して文字列型を生成しています(type {{xmlReferenceName}} string)。同じ入力に対して言語ごとに違う型が返るのは、「同じスキーマから複数言語のクライアントを生成する」というこのリポジトリの目的に照らして、最も避けたい不整合です。

修正

parseTagValue: false を指定し、値を一切変換しません。

要素ごとに個別対処する案は採りませんでした。どの要素が数値に見えるかは入力データ次第で、IDValue のように値によって変換されたりされなかったりします。入力に依存して型が変わるほうが、常に文字列であるより扱いにくいためです。

判断は ADR-0007 に記録しました。

後方互換性

これは出力を変える変更です。 これまで数値だった値が文字列になります。ただし壊れる側が正しい出力なので受け入れます。生成される型宣言と実際の値がこれで一致します。

範囲外(既知)

fast-xml-parser v5 は既定で ignoreAttributes: true のため、refname / shortname / datestamp といった ONIX の属性はそもそも読まれていません。これも実装上の欠落ですが、修正すると出力の形が変わるので別途扱います。ADR-0007 に記録しています。

確認

  • 上表の測定を実際に実行(ONIX 相当の要素を含む XML を両設定でパース)
  • テンプレートと生成物がバイト一致することを diff で確認
  • TypeScript reader は CI で一切実行されていないため、回帰を検出する仕組みはありません

🤖 Generated with Claude Code

https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z


Generated by Claude Code

fast-xml-parser converts anything that looks numeric, which corrupts ONIX
data. Measured:

  default                {"NotificationType":1,"IDValue":62124983,"Price":1200.5}
  parseTagValue:false    {"NotificationType":"01","IDValue":"062124983","Price":"1200.50"}

The code "01" becomes 1, an identifier loses its leading zero, and a
price loses a digit. None of this is new in v5 — v3 did the same — so it
has been wrong from the start.

The generated code already disagreed with it: code.ts declares
`export type NotificationType = string` while the reader returned a
number, so the types were lying. The Go templates generate string types
from the same schema, so the two languages also disagreed with each other
on identical input, which is the one thing a multi-language generator
should not do.

Set parseTagValue: false. Every ONIX element value is a string; the
numeric-looking ones are codes, identifiers and amounts that happen to be
made of digits. Fixing this per-element was rejected in ADR-0007: which
elements look numeric depends on the data, so IDValue would convert or
not depending on the value, and a type that varies with input is worse
than one that is always a string.

This does change output for consumers, from number to string. Recorded as
such — the side that breaks is the side that was wrong.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z

kogai commented Sep 13, 2026

Copy link
Copy Markdown
Owner Author

レビュー

コード変更そのものは正しく、そのままマージできる品質です。 parseTagValue: false は主張どおりの効果があり、報告された変換表は fast-xml-parser 5.11.1 で完全に再現しました。テンプレートと生成物はバイト一致 (sha256 一致)、tsc --noEmit はリポジトリの tsconfig.json でエラーなしです。属性が既定で読まれていないという「範囲外」の主張も実測で裏が取れました。一方で ADR-0007 の Go に関する記述には、実測と食い違う箇所が 2 つあります。 ADR は Accepted の決定記録として残るものなので、ここは直してからマージしてほしいです。修正はいずれも文章のみで、コードには手を入れる必要がありません。


必須 1: ADR-0007 の「Go は一貫して文字列型」は事実と異なる

ファイル: docs/adr/0007-do-not-coerce-onix-values-to-numbers.md(PR 本文にも同じ記述あり)

Go 側の生成コードは、同じスキーマから一貫して文字列型を生成している (type {{xmlReferenceName}} string)。

template/go/v2/code.mustache は 4 分岐で、type X string になるのは {{^spaceSeparatable}} かつ {{^hasElements}} のときだけです。hasElements が真なら struct を、spaceSeparatable なら []string を生成します。

{{#hasElements}}
type {{xmlReferenceName}} struct {
	Body string `xml:",innerxml" json:",omitempty"`
	{{xmlReferenceName}} {{typeName}} `xml:"{{shortname}},attr,omitempty" ...`
}
{{/hasElements}}

実際の生成物 generated/go/v2/code.go の内訳は struct 107 / string 8 / []string 2 です。plain string なのは DateOrDateTime NonEmptyString SourceTypeCode TextCaseCode TextFormatCode TransliterationCode LanguageList74 Sourcename の 8 つだけで、いずれも属性用の型です。

しかも ADR がこの節の直前で例に挙げている NotificationType 自身が Go では struct ですgenerated/go/v2/code.go:18284)。

type NotificationType struct {
	Body string `xml:",innerxml" json:",omitempty"`
	Textformat TextFormatCode `xml:"textformat,attr,omitempty" json:",omitempty"`
	Textcase TextCaseCode `xml:"textcase,attr,omitempty" json:",omitempty"`
	...
}

なぜ問題か: 「Go に合わせる」が決定の根拠として書かれているのに、その Go の実態が違います。この ADR を将来の判断材料に使う人が誤った前提を引き継ぎます。

修正案: この段落を実態に書き換える(「Go は code 型を struct / string / []string に振り分けており、コード値本体は struct の Body string フィールドに入る。いずれにせよ数値化はしない」など)か、Go への言及自体を落とす。この PR は Go を持ち出さなくても十分に成立します(「宣言が string なのに number が返る」という TypeScript 内部で完結した論拠だけで通ります)。


必須 2: 値も一致しないので「言語間の一貫性」は本 PR では達成されない

ファイル: docs/adr/0007-do-not-coerce-onix-values-to-numbers.md

同じ入力に対して言語ごとに違う型が返るのは、「同じスキーマから複数言語のクライアントを生成する」というこのリポジトリの目的に照らして不整合である。

Go の UnmarshalXML は、コード値を人間可読な説明文に置換します(generated/go/v2/code.go。テンプレートは template/go/v2/code.mustachecase "{{value}}" 分岐で {{description}} をバッククォート文字列として代入している)。

	switch v {
  // Use for a complete record issued earlier than approximately six months before publication
  case "01":
		c.Body = `Early notification`
  case "02":
		c.Body = `Advance notification (confirmed)`

つまり同じ <NotificationType>01</NotificationType> に対して、この PR の後でも TypeScript は "01"、Go は "Early notification" を返します。型が揃っても値が揃わないので、「言語間で同じものが返るようにする」という整合性は達成されません。

なぜ問題か: 決定の主要な根拠のひとつが、検証すると成り立ちません。放置すると「TS と Go は揃ったはず」という誤解が残ります。

修正案: 一貫性の主張を撤回し、「TS の型宣言と実際の返り値を一致させる」という範囲に論拠を限定する。Go との差(コード→説明文の変換の有無)に触れるなら、「これは別の未整合として残る」と明記する。


推奨 1: code.ts はどこからも参照されておらず、model.ts は空 interface だけ

ファイル: docs/adr/0007-...md の「結果」節 / generated/typescript/v2/model.ts

ADR は「生成される型宣言 (= string) と実際の値がこれで一致する」と結んでいますが、型検査でこれが担保されるわけではありません。

  • generated/typescript/v2/code.ts は 116 件すべてが export type X = stringnumber/boolean は 0 件)。ここは PR の主張どおりで正しいです。
  • ただし code.tsmodel.ts からも reader.ts からも import されていません(3 ファイルとも import 行ゼロ、model.ts 内に NotificationType の出現も 0 件)。
  • reader.ts の戻り値型 ONIXMessage の実体は generated/typescript/v2/model.ts:71export interface ONIXMessage {} です。model.ts は 75 個の interface すべてがexport interface Price {} など)。

なぜ問題か: 「型が嘘をついていた/これで一致する」という書き方だと、型で守られるようになったと読めます。実際には {} 型なので、変更前も型エラーは出ていませんでしたし、変更後も型検査は何も保証しません。ADR の価値は下がりませんが、記述の正確さは落ちます。

修正案: 「結果」節に一行足す。例:「なお code.ts の型は現時点で model.ts に配線されておらず、reader の戻り値型 ONIXMessage は空 interface である。したがってこの一致は規約上のものであり、型検査では担保されない。」


推奨 2: parseAttributeValue の扱いを一行残す

ファイル: docs/adr/0007-...md の「結果」節(属性の段落)

属性が範囲外という判断は正しく、実測でも裏が取れました。ただし将来属性を有効化する人向けに、もう一段の落とし穴を書いておくと安全です。実測結果:

  • v5 の parseAttributeValue は既定 falseignoreAttributes: false だけを付けた場合、datestamp="20240101""20240101"(文字列)のまま。
  • parseAttributeValue: true にすると "@_d": 20240101"@_n": 1 と、タグ値と同じ壊れ方をする。

なぜ問題か: 「属性を読めるようにする」PR で parseAttributeValue: true を素朴に足すと、この ADR が潰したのと同じバグを属性側で再発させます。

修正案:「属性を読むようにする際は parseAttributeValue を既定の false のままにすること。true にするとタグ値と同じ変換が属性値に起きる。」を追記。


推奨 3: 存在しない AGENTS.md を典拠にしている

ファイル: docs/adr/0007-...md

「同じスキーマから複数言語のクライアントを生成する」というこのリポジトリの目的 (AGENTS.md)

AGENTS.md はこのブランチにも main にも存在しません。全リモートブランチを走査したところ、未マージの origin/claude/agents-md-onix-support-c9y6cu にのみありました。

なぜ問題か: リンク切れの典拠です。そのブランチがマージされない/別名になると、根拠が辿れなくなります。

修正案: README.md の "Language Agnostic, which should be easy to addition new programming language" を典拠にする。あるいは(必須 2 を採るなら)この文ごと落とす。


任意 1: 「一切変換しない」は言い過ぎ(trim は残る)

parseTagValue: false を入れても trimValues は既定 true のままなので、前後の空白は落ちます。実測:

<Pad>  0123  </Pad>  ->  "0123"     (前後空白は除去される)
<Ws>   </Ws>         ->  ""

ONIX では望ましい挙動だと思いますが、ADR の「要素の値を一切変換しない」という表現は正確ではありません。「数値・真偽値への変換を行わない(空白の trim は従来どおり)」程度にすると正確です。なお空要素 <Empty></Empty>・自己閉じ <SelfClose/>・混在内容 <Mixed>text <b>bold</b> tail</Mixed> は変更前後で完全に同一の結果になることも確認済みで、副作用は見つかりませんでした

任意 2: 真偽値の変換も止まっている(ADR は数値にしか触れていない)

既定では <TrueV>true</TrueV>boolean true になりますが、parseTagValue: false でこれも "true"(文字列)になります。ADR のタイトルと本文は「数値」に限定していますが、実際の効果は真偽値にも及びます。ONIX に true/false を取る要素は多くないとはいえ、一行記録しておくと後で驚かずに済みます。

任意 3: 「入力依存で型が変わる」の実例を補強できる

ADR の「IDValue のように値によって変換されたりされなかったりする」という指摘は正しく、しかも桁数でも揺れます。v5 既定での実測:

"11111111111111111111"    (20桁) -> string "11111111111111111111"
"12345678901234567890123" (23桁) -> number 1.2345678901234568e+22
"9784062124983"           (13桁) -> number 9784062124983

同じ「数字だけの文字列」でも string / number / 精度が落ちた number が混在します。個別対処を採らないという判断を補強する良い実例なので、書き足す価値があります。


検証したこと(コマンドと出力)

作業ツリーには一切触れていません。すべて git show による読み取りと、リポジトリ外の一時ディレクトリでの実行です。

1. 変換表の再現(fast-xml-parser 5.11.1、package.json の指定バージョン)

const base = { ignoreDeclaration: true, ignorePiTags: true };
// <NotificationType>01</...><ProductIDType>02</...><IDValue>062124983</...>
// <ISBN13>9784062124983</...><Price>1200.50</...>
DEFAULT (親ブランチ相当)
  {"NotificationType":1,"ProductIDType":2,"IDValue":62124983,"ISBN13":9784062124983,"Price":1200.5}
  types: 全部 number
parseTagValue:false (本 PR)
  {"NotificationType":"01","ProductIDType":"02","IDValue":"062124983","ISBN13":"9784062124983","Price":"1200.50"}
  types: 全部 string

PR / ADR の表と完全一致。parseTagValue: false は 5 値すべてを完全に解決しており、型が変わり残る値はありません。

2. 副作用の確認(親ブランチ設定との比較)

入力 既定 parseTagValue:false
<Empty></Empty> "" ""(同じ)
<SelfClose/> "" ""(同じ)
<Ws> </Ws> "" ""(同じ)
<Mixed>text <b>bold</b> tail</Mixed> {"b":"bold","#text":"texttail"} 同一
<TrueV>true</TrueV> boolean true "true"
<Pad> 0123 </Pad> number 123 "0123"(trim される)
<Hex>0x1A</Hex> number 26 "0x1A"
<Exp>1e5</Exp> number 100000 "1e5"
<Plus>+42</Plus> number 42 "+42"
<Date>20240101</Date> number 20240101 "20240101"

→ 形が変わるのは値の型だけ。空要素・空白のみ・混在内容の構造は不変。PR が触れていない差分は「真偽値も止まる」「trim は残る」の 2 点のみ(任意 1・2)。

3. 属性(PR の「範囲外」主張の検証)

DEFAULT (ignoreAttributes 既定)
 {"ONIXMessage":{"Header":{"Sender":{"SenderName":"X"}},"Product":{"NotificationType":1}}}

refname / shortname / datestamp / release はすべて結果から消えています。PR の主張どおりです。 ignoreAttributes: false を付けると初めて @_refname 等が現れ、そのとき @_datestamp は既定で文字列のまま(parseAttributeValue 既定 false)、parseAttributeValue: true にすると 20240101(number)になります。

4. v3 の挙動(ADR の「最初から壊れていた」)— fast-xml-parser 3.17.6 を併置

v3 DEFAULT (parse(xml), オプション無し)
  {"NotificationType":1,"ProductIDType":2,"IDValue":62124983,"ISBN13":9784062124983,"Price":1200.5,"TrueV":true}
v3 parseTrueNumberOnly:false
  同上(=既定と同一)
v3 parseNodeValue:false
  {"NotificationType":"01",...,"Price":"1200.50","TrueV":"true"}
v5 numberParseOptions.leadingZeros:true
  {"A":1,"B":62124983,"C":9784062124983,"D":1200.5}   ← v5 既定と同一

git show origin/main:template/typescript/v2/reader.mustachexml.parse(file.toString()) でオプションを一切渡していません(=v3 既定)。→ 「v3 でも同じ」「最初から壊れていた」「parseTrueNumberOnly:false と v5 の leadingZeros:true が同じ結果」はいずれも正しいです。ちなみに v5 で leadingZeros:false にすると "01"/"062124983" は救われますが Price1200.5 のまま — 部分的対処では足りないという ADR の判断も裏付けられます。

5. テンプレートと生成物のバイト一致

$ diff <(git show $B:template/typescript/v2/reader.mustache) \
       <(git show $B:generated/typescript/v2/reader.ts)   # 差分なし
333192663d60f5607eca6528ec4bd7864570f6fb178c46edd2e516927e566be4  (両方)

6. 型検査 — 隔離ディレクトリに generated/typescript/v2/*.tstsconfig.json を複製し、typescript@5 + @types/node@22.20.2 + fast-xml-parser@5.11.1 で実行:

$ tsc --noEmit -p tsconfig.json
exit=0

7. 生成された型の確認

$ git show $B:generated/typescript/v2/code.ts | grep -c '^export type .* = string'   # 116
$ ... | grep -cE '= *number|= *boolean'                                              # 0
$ git show $B:generated/typescript/v2/model.ts | grep -cE '^export interface \w+ \{\}$'  # 75 / 全 75
$ git show $B:generated/go/v2/code.go | grep -E '^type ' | awk '{print $3}' | sort | uniq -c
    107 struct
      8 string
      2 []string

8. CI 適用範囲.github/workflows/test.ymlmake test(Haskell)と npx bazelisk test //e2e/go:snapshot_test のみ。e2e/ 配下は e2e/go だけで、TypeScript の reader を実行するテストはリポジトリ内に存在しません。→ 「TypeScript reader は CI で実行されていない」は正しいです。ADR がこれを正直に明記している点は良いと思います。

9. ADR 周辺docs/adr/0005-...md は確かに本件を「既知の未解決問題」として記録しており、parseTagValue: false 相当の対処が要ると明記しています。参照は正確です。ADR 番号 0007 は他ブランチ(0003 / 0004 / 0005 / 0006 が使用中)と衝突しません。

検証できなかったこと

  • 実 ONIX ファイル (fixtures/20201200.onix) での変更前後の全量比較は行っていません。作業ツリーを触らない制約のため、合成した最小 XML での検証に留めています。変換の性質上、結論は変わらないと考えていますが、「実データで数値化されていた要素が具体的にどれだけあるか」の件数は不明です。
  • 既存利用者への実影響は測っていません。README.md 上 TypeScript は Version 2 / 3 とも "Not yet" で、npm 公開物としての利用実績を確認する手段がありませんでした。破壊的変更を受け入れるという判断自体には異論ありません。
  • Go 側の実行時挙動UnmarshalXML が実際に説明文を返すこと)は生成コードの読解による確認で、Go を実行しての確認はしていません。ただし該当箇所(case "01"Early notification を代入)は分岐が単純で、読解で十分と判断しました。

Generated by Claude Code

The ADR argued the fix restores cross-language consistency because Go
generates string types from the same schema. Review showed both halves of
that are wrong, and checking confirmed it:

  generated/go/v2/code.go: 107 struct, 8 string, 2 []string

`type X string` only comes out of the template when a code is neither
space-separatable nor has attributes, and NotificationType — the ADR's own
example — is a struct in Go.

Worse for the argument, Go's UnmarshalXML swaps the code for its
description:

	case "01":
		c.Body = `Early notification`

So after this change TypeScript returns "01" and Go returns "Early
notification". The languages still disagree; the fix does not touch that.
That is a bigger design split — only Go expands codes — and belongs in its
own decision.

The justification is narrowed to the thing that does hold: "01" becoming
1 and "1200.50" becoming 1200.5 corrupts data, whatever Go does. The code
change is unaffected.

Also recorded: parseAttributeValue must stay false when attributes are
eventually read, or the same bug returns on that side; the generated
types agreeing with the values is a convention rather than something
typecheck enforces, since every interface in model.ts is empty and
code.ts is imported by nothing; and parseTagValue: false does not stop
trimValues, which still strips surrounding whitespace.

The AGENTS.md citation is replaced with README, since AGENTS.md is not on
this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P8rZakwAiz1jpViUX34x1Z

kogai commented Sep 13, 2026

Copy link
Copy Markdown
Owner Author

レビューありがとうございます。必須 2 件はどちらも正しく、ADR-0007 の論拠が誤っていました。 訂正しました (7332d96)。

必須 1・2: Go 側の実態

自分でも確認しました。

$ grep -cE '^type [A-Za-z0-9]+ struct'   generated/go/v2/code.go   → 107
$ grep -cE '^type [A-Za-z0-9]+ string$'  generated/go/v2/code.go   →   8
$ grep -cE '^type [A-Za-z0-9]+ \[\]string$' generated/go/v2/code.go →   2

type X string になるのは spaceSeparatable でも hasElements でもない場合だけで、ADR が例に挙げていた NotificationType は Go では struct でした(code.go:18284)。

さらに決定的なのがこちらです。

	switch v {
	// Use for a complete record issued earlier than ...
	case "01":
		c.Body = `Early notification`

Go はコード値を人間可読な説明文に置換していました。 つまりこの PR を入れても TypeScript は "01"、Go は "Early notification" を返し、言語間の一貫性は回復しません。「型は揃うが値は揃わない」というご指摘のとおりです。

対応

ADR の根拠を**「データを壊さないこと」の一点に絞り**ました。"01"1"1200.50"1200.5 はそれ自体がデータ破壊であり、Go が何をしていようと直すべきものです。コード変更自体は影響を受けません。

そのうえで、Go だけがコードを説明文に展開しているという食い違いを、より大きな設計上の問題として ADR に明記しました。これは型の問題ではなく「どの言語でコードを展開するか」の設計判断なので、別途 ADR を立てて扱うべきものだと考えています。このリポジトリの価値の中心(同じスキーマから複数言語)に直接関わる論点を掘り出していただいた形です。

推奨

  • parseAttributeValue を既定の false のまま保つこと: 属性を読むようにしたときに同じバグが属性側で再発する、というのはそのとおりです。ADR の「結果」に明記しました。
  • 型の一致は規約であって検査されていない: code.ts がどこからも import されておらず、model.ts の 75 個の interface がすべて空(export interface ONIXMessage {})なので、as ONIXMessage は何も保証しません。ADR にそのまま記載しました。「型宣言と値が一致する」と書くなら、この限界も併記すべきでした。
  • AGENTS.md はこのブランチに無い: Add AGENTS.md #55 の未マージブランチにしか存在しません。README の "Language Agnostic" を参照するよう直しました。

任意

  • 「値の加工を一切しない」は言い過ぎ: trimValues は既定 true のままなので前後の空白は落ちます。真偽値らしき文字列の変換も同時に止まる点とあわせて追記しました。
  • v5 の既定が桁数で型を変える(20 桁は string、23 桁は 1.23e+22): ADR の「入力に依存して型が変わる」という論拠を補強する材料なので、指摘として受け止めています。

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant