Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions TODO.sts-refactor/00-overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
|---|------|----------|----------|--------------|
| 01 | `01-mathml-delegation.md` | Anti-Pattern Fix | DONE | — |
| 02 | `02-type-resolution.md` | Anti-Pattern Fix | DONE | — |
| 03 | `03-namespace-coupling.md` | Architecture | IN PROGRESS | 01, 02 |
| 03 | `03-namespace-coupling.md` | Architecture | IN PROGRESS (63→20 refs) | 01, 02 |
| 04 | `04-register-versioning.md` | Architecture | HIGH | 01, 02 |
| 05 | `05-missing-elements.md` | Feature Gap | MOSTLY DONE | 04 |
| 06 | `06-missing-attributes.md` | Feature Gap | DONE | 04 |
Expand Down Expand Up @@ -140,7 +140,9 @@ grep -r "method_missing|respond_to_missing|Object.const_get|\.send" lib/
```

### IsoSts Namespace Independence (2026-05-07)
- ARCHITECTURE DECISION: IsoSts and NisoSts must remain fully independent
- ARCHITECTURE DECISION: IsoSts must be independent **of NisoSts**. This is not
independence in general — `lib/sts/iso_sts/` still references `TbxIsoTml` and
MathML types directly, and those shared namespaces are outside this ADR.
- ISOSTS is frozen legacy; NISO STS evolves — combining them violates OCP
- Created 11 new IsoSts-specific types: Monospace, Sc, Strike, Underline, Uri,
NamedContent, StandardRef, MetaDate, ContentLanguage (with autoloads)
Expand All @@ -164,9 +166,27 @@ grep -r "method_missing|respond_to_missing|Object.const_get|\.send" lib/
- ~~Expand StringName usage (in contrib, element-citation, related-article — NISO STS 1.2)~~ → DONE: added to name-alternatives; already in person-group and mixed-citation

### Architectural Items (High Effort)
- `03-namespace-coupling.md` — IN PROGRESS: 157→63 IsoSts→NisoSts cross-references remaining
- `03-namespace-coupling.md` — IN PROGRESS: 63→20 IsoSts→NisoSts references
remaining (issue #40). Source of truth is `ISOSTS.xsd`, NOT the NisoSts
models — they disagree on nearly every element.
- `04-register-versioning.md` — Version the models via lutaml-model Registers
- `11-duplicate-models.md` — 44 overlapping element resolution (depends on 03)

### Known bugs — schema conformance (separate from 03)
The 2026-05-07 "@id added to all models (XSD-verified)" pass verified against
the **NISO** XSD and applied the result to IsoSts. ISOSTS disagrees:
- **16 IsoSts models silently drop ISOSTS attributes that exist** — real data
loss. `sub`/`sup` lose `arrange`+`specific-use`; `ext-link` loses 5 xlink
attrs; `mixed-citation` loses 6; `graphic`, `copyright-*`, `edition`,
`title`, `label`, `uri`, `named-content`, `underline`, `meta-date`, `body`
lose others.
- **25 IsoSts models carry an `@id` ISOSTS does not define** — harmless (never
populated on parse, never emitted on serialise), but dead surface.

## Next Action
Proceed with `03-namespace-coupling.md` — audit and resolve 157+ IsoSts→NisoSts cross-references.
Two independent tracks:
1. Finish `03` — the 20 remaining refs split into 11 deep-root refs (each
reaching a ~152-class recursive core) and 9 child-bearing refs whose closure
is unmeasured. Both need sizing before planning.
2. File and fix the 16 lossy models above — real data loss, unrelated to
decoupling.
255 changes: 108 additions & 147 deletions TODO.sts-refactor/03-namespace-coupling.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,156 +3,117 @@
**Priority**: HIGH
**Category**: Architecture
**Estimated Effort**: High
**Files Affected**: `lib/sts/iso_sts/content_groups/highlight_elements.rb` (194+ references)
**Status**: Partially done — 63 → 20 references (GitHub issue #40)

## Problem

`lib/sts/iso_sts/content_groups/highlight_elements.rb` contains 194+ references to `Sts::NisoSts::*` types:

```ruby
# Examples from highlight_elements.rb
[:sub, "Sts::NisoSts::Sub", "sub"]
[:sup, "Sts::NisoSts::Sup", "sup"]
[:italic, "Sts::NisoSts::Italic", "italic"]
[:bold, "Sts::NisoSts::Bold", "bold"]
# ... 190 more
```

### Why This Is Wrong

1. **Violation of namespace isolation**: IsoSts should not depend on NisoSts implementation
2. **Shared-namespace ambiguity**: These elements appear in both ISOSTS and NISO STS but with different attribute sets
3. **Tight coupling**: Changes to NisoSts can break IsoSts
4. **Schema divergence ignored**: `<p>` in ISOSTS has different attributes than `<p>` in NISO STS Extended

### Root Cause

The schemas ARE different:
- ISOSTS `<p>`: `@content-type, @id, @specific-use, @xml:lang`
- NISO STS Extended `<p>`: `@content-type, @id, @originator, @specific-use, @style-type, @xml:base, @xml:lang`

## Solution: Three-Tier Architecture

### Tier 1: Shared Base Types

Create shared types in `Sts::Base` that both namespaces inherit:

```ruby
# lib/sts/base/text/emphasis.rb
module Sts
module Base
module Text
class Emphasis < Lutaml::Model::Serializable
# Shared attributes/methods
attribute :id, :string
attribute :base_fontsize, :string
attribute :baseline_shift, :string
attribute :color, :string

xml do
map_attribute :id, to: :id
# ...
end
end

class Bold < Emphasis
xml { element "bold" }
end

class Italic < Emphasis
xml { element "italic" }
end

class Sub < Emphasis
xml { element "sub" }
end

class Sup < Emphasis
xml { element "sup" }
end
end
end
end
```

### Tier 2: IsoSts-Specific Extensions

```ruby
# lib/sts/iso_sts/text/emphasis.rb
module Sts
module IsoSts
module Text
class Bold < ::Sts::Base::Text::Bold
# Add IsoSts-specific attributes
attribute :content_type, :string

xml do
# Override/add IsoSts-specific mapping
map_attribute "content-type", to: :content_type
end
end
end
end
end
```

### Tier 3: NisoSts-Specific Extensions

```ruby
# lib/sts/niso_sts/text/emphasis.rb
module Sts
module NisoSts
module Text
class Bold < ::Sts::Base::Text::Bold
# Add NisoSts-specific attributes
attribute :originator, :string
attribute :style_type, :string

xml do
map_attribute "originator", to: :originator
map_attribute "style-type", to: :style_type
end
end
end
end
end
```

## Implementation Strategy

1. **Phase 1**: Identify all shared element types (bold, italic, sub, sup, p, etc.)
2. **Phase 2**: Create base types with MINIMAL shared attributes
3. **Phase 3**: Refactor IsoSts to extend bases
4. **Phase 4**: Refactor NisoSts to extend bases
5. **Phase 5**: Delete `Sts::NisoSts::*` references from IsoSts content groups

## Files to Refactor

Primary targets:
- `lib/sts/iso_sts/content_groups/highlight_elements.rb`
- `lib/sts/iso_sts/content_groups/emphasis_elements.rb` (if exists)
- `lib/sts/niso_sts/content_groups/highlight_elements.rb`
`lib/sts/iso_sts/` references `Sts::NisoSts::*` types directly. IsoSts must be
independent of NisoSts (ADR 2026-05-07): ISOSTS is frozen legacy, NISO STS
evolves, so coupling them violates OCP.

PR #31 reduced this from 157 to 63 references. Issue #40 reduced it further,
from 63 to 20.

## Source of truth: ISOSTS.xsd, NOT the NisoSts models

The obvious approach — duplicate each NisoSts class into IsoSts — produces
**schema-incorrect models**. The NisoSts models disagree with
`reference-docs/isosts-v1/xsd/ISOSTS.xsd` on nearly every element:

| Element | ISOSTS says | NisoSts model |
|---|---|---|
| `year` (:948) | `content-type`, `specific-use`, `xml:lang` — no `@id` | `@id` only |
| `doc-type` (:6378) | `type="xs:string"` — no attributes at all | `@id` + content |
| `ics` (:6373) | `type="xs:string"` | `@id` + `ics-desc` child |
| `fpage` | `content-type`, `seq`, `specific-use`, `xml:lang`; no children | `@id` + bold/italic |
| `license` (:51) | `license-type`, `specific-use`, `xml:lang` | `@id`, `xlink:href` |
| `ruby` | **not an ISOSTS element** | exists (NISO only) |

`feature_doc.xml` declares `<!DOCTYPE standard SYSTEM ".../ISOSTS.dtd">`, and
the ISOSTS DTD agrees with ISOSTS.xsd (both derive `bold`, `sub` etc. from the
JATS 0.4 modules). ISOSTS.xsd is a faithful conversion and is the authority.

**Round-tripping does not prove correctness.** A model that invents or drops an
attribute still parses and serialises symmetrically, so the suite stays green
while the model is wrong. Assert the exact attribute set per element instead.

Attribute lists must be **generated** from the XSD, never hand-read: `version`
on `tex-math` and `specific-use` on `pub-id` sit after long `xs:enumeration`
blocks and are invisible to a truncated read.

## Done in issue #40 — 43 refs removed, 27 classes added

- **14 `xs:string` elements** (`originator`, `doc-type`, `doc-number`,
`part-number`, `version`, `suppl-type`, `suppl-number`, `suppl-version`,
`urn`, `sdo`, `proj-id`, `release-version`, `ics`, `secretariat`) — modelled
as content-only IsoSts classes with no attributes. 23 refs.
- **3 `permissions` refs** repointed to the existing `IsoSts::Permissions`.
- **`ruby` deleted** from `StyledContent` — ISOSTS defines no such element.
Behaviour change: `<ruby>` in `<styled-content>` no longer round-trips.
- **13 element classes** modelled from ISOSTS.xsd: `Year`, `PubDate`,
`ReleaseVersionId`, `IsProof`, `AltText`, `LongDesc`, `TexMath`, `PubId`,
`Volume`, `Issue`, `Fpage`, `Lpage`, `PageRange`. 16 refs.
- **`WiNumber` added** — ISOSTS gives `wi-number` an `@id`; it was typed as a
plain string, silently discarding that `@id`.

### Why classes and not plain `:string`

`xs:string` elements look like they need no class. They do: an empty element
(`<sdo/>`, which `feature_doc.xml` contains) does not survive a `:string`
round-trip. For a scalar, `render_empty: :empty` recovers it; for a collection
(`secretariat`, `ics` — both `maxOccurs="unbounded"`) an empty element parses
to `[]`, destroying the information at parse time before any render option
applies. Content-only classes round-trip every case.

## Remaining — 20 refs

**11 refs to 9 deep roots**: `MetadataStd`, `ElementCitation`, `PersonGroup`,
`Collab`, `Source`, `DispQuote`, `EditingInstruction`, `TermDisplay`,
`BoxedText`. Each reaches the same ~152-class mutually-recursive core
(`Section` → `Paragraph` → `DispQuote` → `Paragraph`). `DispQuote` has 3 direct
children but pulls all 152. No incremental path — needs its own decision.

**9 refs whose ISOSTS content models have real children**, closure not measured:
`attrib` (:1082), `term-head` (:4308), `article-title`, `license` → `license-p`
(:51), `publisher` → `publisher-loc`, `custom-meta-group` → `custom-meta`.
`attrib` and `term-head` are **not** trivial leaves in ISOSTS despite what the
NisoSts models suggest; `license-p` reaches `array`/`alternatives`, so these may
not be cheap. Measure before planning.

## Rejected: three-tier `Sts::Base` hierarchy

An earlier draft of this document proposed shared base types in `Sts::Base` that
both namespaces inherit. This contradicts the 2026-05-07 ADR, and the schemas
genuinely diverge — `IsoSts::Fig` has `title`/`alternatives` that
`NisoSts::Figure` lacks; `IsoSts::Ref` has `nlm-citation`/`citation-alternatives`
that `NisoSts::Reference` lacks. A shared base would fight the schemas.

That draft also targeted `lib/sts/iso_sts/content_groups/highlight_elements.rb`
and "194+ references" to emphasis types. That file was deleted as dead code; the
real work was metadata types, not highlight elements.

## Naming trap (for the deferred work)

A mechanical `s/NisoSts::/IsoSts::/` is wrong. Nine elements already have
different class names per namespace, so a blind rename creates `IsoSts::Section`
alongside the existing `IsoSts::Sec` — two classes for one `sec` element:

`Section`/`Sec`, `Figure`/`Fig`, `Reference`/`Ref`, `ReferenceList`/`RefList`,
`SectionArray`/`Array`, `StdCrossReference`/`StandardCrossReference`,
`DisplayFormula`/`DispFormula`, `TermSection`/`TermSec`,
`ReferenceStandard`/`Std`.

Remap child references by **XML element name**, not class name.

## Scope boundary

Independence from NisoSts is not independence in general: `lib/sts/iso_sts/`
still references 6 `TbxIsoTml` types directly, and much of the deferred closure
reaches `TbxIsoTml`/MathML. Those are shared namespaces, outside this ADR.

## Verification

1. No `Sts::NisoSts` references in `lib/sts/iso_sts/`
2. All round-trip tests pass
3. Schema validation against both ISOSTS DTD and NISO STS XSD passes

## Dependencies

- `01-mathml-delegation.md` — uses similar patterns
- `02-type-resolution.md` — must be clean first

## TODO Checklist

- [ ] Audit all IsoSts → NisoSts references
- [ ] Categorize by shared vs. truly namespace-specific
- [ ] Design base type hierarchy
- [ ] Create `lib/sts/base/` directory structure
- [ ] Implement base types for shared elements
- [ ] Refactor IsoSts to use base types
- [ ] Refactor NisoSts to use base types
- [ ] Remove cross-namespace references
- [ ] Verify no circular dependencies
- [ ] Run full test suite
1. `grep -rho "Sts::NisoSts::" lib/sts/iso_sts/ | wc -l` → `20`
2. Every attribute on an IsoSts model traces to a line in `ISOSTS.xsd`
3. Autoload registry 1:1 with the directory (112/112)
4. `bundle exec rspec` green; `bundle exec rubocop` clean
27 changes: 27 additions & 0 deletions lib/sts/iso_sts.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,33 @@ module IsoSts
autoload :MetaDate, "#{__dir__}/iso_sts/meta_date"
autoload :ContentLanguage, "#{__dir__}/iso_sts/content_language"
autoload :StandardRef, "#{__dir__}/iso_sts/standard_ref"
autoload :WiNumber, "#{__dir__}/iso_sts/wi_number"
autoload :Originator, "#{__dir__}/iso_sts/originator"
autoload :DocType, "#{__dir__}/iso_sts/doc_type"
autoload :DocNumber, "#{__dir__}/iso_sts/doc_number"
autoload :PartNumber, "#{__dir__}/iso_sts/part_number"
autoload :Version, "#{__dir__}/iso_sts/version"
autoload :SupplType, "#{__dir__}/iso_sts/suppl_type"
autoload :SupplNumber, "#{__dir__}/iso_sts/suppl_number"
autoload :SupplVersion, "#{__dir__}/iso_sts/suppl_version"
autoload :Urn, "#{__dir__}/iso_sts/urn"
autoload :Sdo, "#{__dir__}/iso_sts/sdo"
autoload :ProjId, "#{__dir__}/iso_sts/proj_id"
autoload :ReleaseVersion, "#{__dir__}/iso_sts/release_version"
autoload :Ics, "#{__dir__}/iso_sts/ics"
autoload :Year, "#{__dir__}/iso_sts/year"
autoload :PubDate, "#{__dir__}/iso_sts/pub_date"
autoload :ReleaseVersionId, "#{__dir__}/iso_sts/release_version_id"
autoload :IsProof, "#{__dir__}/iso_sts/is_proof"
autoload :AltText, "#{__dir__}/iso_sts/alt_text"
autoload :LongDesc, "#{__dir__}/iso_sts/long_desc"
autoload :TexMath, "#{__dir__}/iso_sts/tex_math"
autoload :PubId, "#{__dir__}/iso_sts/pub_id"
autoload :Volume, "#{__dir__}/iso_sts/volume"
autoload :Issue, "#{__dir__}/iso_sts/issue"
autoload :Fpage, "#{__dir__}/iso_sts/fpage"
autoload :Lpage, "#{__dir__}/iso_sts/lpage"
autoload :PageRange, "#{__dir__}/iso_sts/page_range"

# Section elements
autoload :Sec, "#{__dir__}/iso_sts/sec"
Expand Down
23 changes: 23 additions & 0 deletions lib/sts/iso_sts/alt_text.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

module Sts
module IsoSts
class AltText < Lutaml::Model::Serializable
attribute :content, :string
attribute :id, :string
attribute :content_type, :string
attribute :specific_use, :string
attribute :xml_lang, :string

xml do
element "alt-text"

map_content to: :content
map_attribute "id", to: :id
map_attribute "content-type", to: :content_type
map_attribute "specific-use", to: :specific_use
map_attribute "xml:lang", to: :xml_lang
end
end
end
end
2 changes: 1 addition & 1 deletion lib/sts/iso_sts/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class App < Lutaml::Model::Serializable
attribute :term_sec, ::Sts::IsoSts::TermSec, collection: true
attribute :fn_group, ::Sts::IsoSts::FnGroup, collection: true
attribute :ref_list, ::Sts::IsoSts::RefList, collection: true
attribute :permissions, ::Sts::NisoSts::Permissions
attribute :permissions, ::Sts::IsoSts::Permissions
attribute :non_normative_note, ::Sts::IsoSts::NonNormativeNote,
collection: true
attribute :non_normative_example, ::Sts::IsoSts::NonNormativeExample,
Expand Down
2 changes: 1 addition & 1 deletion lib/sts/iso_sts/array.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Array < Lutaml::Model::Serializable
attribute :label, ::Sts::IsoSts::Label
attribute :table, ::Sts::IsoSts::Table
attribute :attrib, ::Sts::NisoSts::Attrib, collection: true
attribute :permissions, ::Sts::NisoSts::Permissions, collection: true
attribute :permissions, ::Sts::IsoSts::Permissions, collection: true

xml do
element "array"
Expand Down
2 changes: 1 addition & 1 deletion lib/sts/iso_sts/disp_formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class DispFormula < Lutaml::Model::Serializable
attribute :styled_content, ::Sts::IsoSts::StyledContent
attribute :preformat, ::Sts::IsoSts::Preformat
attribute :graphic, ::Sts::IsoSts::Graphic, collection: true
attribute :tex_math, ::Sts::NisoSts::TexMath
attribute :tex_math, ::Sts::IsoSts::TexMath

xml do
element "disp-formula"
Expand Down
Loading
Loading