Audit follow-ups: TODO #1 (alias spec), #2 (sharded warning), #3 (kebab-case rename)

lib/opencdd/parcel/scrape_verifier.rb

@@ -4,7 +4,7 @@ module Opencdd
   module Parcel
     # Verifies that scrape output produced populated .xls files, not
     # just schema stubs. Catches the silent failure mode discovered
-    # on 2026-07-08: iec61360/iec63213/iec61360-7 scrapes appeared
+    # on 2026-07-08: iec61360/iec-63213/iec-61360-7 scrapes appeared
     # complete (10,338 PROPERTY .xls files for iec61360) but every
     # file had 0 data rows. The build pipeline exported 574 classes
     # + 0 properties, and the browser showed "No declared properties"

lib/opencdd/parcel/sharded_dir_reader.rb

@@ -26,7 +26,7 @@ module Parcel
     #     +.xls+ files into a Workbook.
     #
     # Example:
-    #   reader = Opencdd::Parcel::ShardedDirReader.new("downloads/iec63213")
+    #   reader = Opencdd::Parcel::ShardedDirReader.new("downloads/iec-63213")
     #   db = Opencdd::Database.new
     #   reader.load_into(db)
     #

lib/opencdd/parcel/versioned_reader.rb

@@ -17,7 +17,7 @@ module Parcel
     #     version's subfolder.
     #
     # Example:
-    #   reader = Opencdd::Parcel::VersionedReader.new("downloads/iec63213")
+    #   reader = Opencdd::Parcel::VersionedReader.new("downloads/iec-63213")
     #   reader.versions_for("KEA012")    # => 3 VersionHistory::Entry
     #   reader.load_version("KEA012", "ABC123...") # => Database with v002 content
     class VersionedReader

spec/det_classification_aliased_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe "DET classification property-ID aliasing" do
   let(:path) do
-    "/Users/mulgogi/src/opencdd/data-private/exports/latest/iec61360-4/" \
+    "/Users/mulgogi/src/opencdd/data-private/exports/latest/iec-61360-4/" \
       "export_DETCLASSIFICATION_DOMO-DWL8BK.xls"
   end
 

spec/det_classification_spec.rb

@@ -55,7 +55,7 @@
 
 RSpec.describe "DET classification importer (end-to-end via search-export)" do
   let(:path) do
-    "/Users/mulgogi/src/opencdd/data-private/exports/latest/iec61360-4/" \
+    "/Users/mulgogi/src/opencdd/data-private/exports/latest/iec-61360-4/" \
       "export_DETCLASSIFICATION_DOMO-DWL8BK.xls"
   end
 

spec/parcel/referenced_irdis_spec.rb

@@ -4,19 +4,19 @@
 require "json"
 
 # Exercises the same sharded per-class fixture as
-# +sharded_dir_reader_spec.rb+ (downloads/iec63213/). KEA011 is the
+# +sharded_dir_reader_spec.rb+ (downloads/iec-63213/). KEA011 is the
 # canonical worked example: it declares 7 applicable_property_irdis
 # (KEA336, KEA337, KEA338, KEA321, KEA322, KEA323, KEA324) all in the
 # 63213 scheme, plus one imported property (0112/2///62683#ACE808) from
 # a different scheme — that cross-dict reference is what proves the
 # source-scheme partitioning works.
 RSpec.describe Opencdd::Parcel::ReferencedIrdis do
-  FIXTURE = File.expand_path("../../downloads/iec63213", __dir__)
+  FIXTURE = File.expand_path("../../downloads/iec-63213", __dir__)
 
   subject(:collector) { described_class.new(FIXTURE) }
 
   before(:all) do
-    skip "sharded fixture downloads/iec63213 not present" unless File.directory?(FIXTURE)
+    skip "sharded fixture downloads/iec-63213 not present" unless File.directory?(FIXTURE)
   end
 
   let(:manifest) { collector.collect }

spec/parcel/scrape_verifier_spec.rb

@@ -6,13 +6,13 @@
   let(:verifier) { described_class.new(directory) }
 
   describe "#verify on a directory with populated .xls files" do
-    let(:directory) { File.expand_path("../../downloads/iec62683", __dir__) }
+    let(:directory) { File.expand_path("../../downloads/iec-62683", __dir__) }
 
     it "returns ok results for CLASS files with data rows", :slow do
-      skip "iec62683 scrape not present locally" unless File.directory?(directory)
+      skip "iec-62683 scrape not present locally" unless File.directory?(directory)
       results = verifier.verify
       class_results = results.select { |r| r.entity_type == "CLASS" }.first(20)
-      skip "no CLASS .xls files in iec62683" if class_results.empty?
+      skip "no CLASS .xls files in iec-62683" if class_results.empty?
       # CLASS files always have data rows; PROPERTY/VALUELIST/VALUETERMS
       # exports may be header-only for some classes (known scrape
       # characteristic — the iec61360 incident).
@@ -21,7 +21,7 @@
   end
 
   describe "#verify on the iec61360 family (regression)" do
-    ["iec61360", "iec63213", "iec61360-7"].each do |dict|
+    ["iec61360", "iec-63213", "iec-61360-7"].each do |dict|
       it "reports whether #{dict} property files have data", :slow do
         dir = File.expand_path("../../downloads/#{dict}", __dir__)
         skip "#{dict} scrape not present locally" unless File.directory?(dir)
@@ -42,8 +42,8 @@
 
   describe "#summary" do
     it "returns a hash with total/ok/empty/by_type counts" do
-      dir = File.expand_path("../../downloads/iec62683", __dir__)
-      skip "iec62683 scrape not present locally" unless File.directory?(dir)
+      dir = File.expand_path("../../downloads/iec-62683", __dir__)
+      skip "iec-62683 scrape not present locally" unless File.directory?(dir)
       summary = described_class.new(dir).summary
       expect(summary).to include(:total, :ok, :empty, :by_type, :empty_by_type)
       expect(summary[:total]).to be_positive

spec/parcel/sharded_dir_reader_spec.rb

@@ -5,19 +5,20 @@
 require "fileutils"
 require "json"
 
-# Exercises the sharded per-class layout in downloads/iec63213/, which
+# Exercises the sharded per-class layout in downloads/iec-63213/, which
 # has 26 class subdirectories (KEA001..KEB124) each containing 4 .xls
 # files (CLASS, PROPERTY, VALUELIST, VALUETERMS). The PROPERTY /
 # VALUELIST / VALUETERMS workbooks currently hold header-only exports,
 # so only the CLASS rows produce entities — that's a scrape-time data
 # characteristic, not a reader bug.
 RSpec.describe Opencdd::Parcel::ShardedDirReader do
-  SHARDED_FIXTURE = File.expand_path("../../downloads/iec63213", __dir__)
+  SHARDED_FIXTURE = File.expand_path("../../downloads/iec-63213", __dir__)
 
   let(:reader) { described_class.new(SHARDED_FIXTURE) }
 
   before(:all) do
-    skip "sharded fixture downloads/iec63213 not present" unless File.directory?(SHARDED_FIXTURE)
+    skip "sharded fixture downloads/iec-63213 not present" unless File.directory?(SHARDED_FIXTURE)
+    warn_poor_fixture(SHARDED_FIXTURE)
   end
 
   describe "#class_subdirs" do
@@ -59,14 +60,14 @@
     end
   end
 
-  # Self-contained layout tests that don't depend on downloads/iec63213
+  # Self-contained layout tests that don't depend on downloads/iec-63213
   # state. Copies a real export_*.xls into the synthetic layout so the
   # reader exercises real XLS parsing, not just file existence checks.
   describe "per-version layout (nested <CODE>/<UNID>/export_*.xls)" do
     let(:fake_unid) { "1FF0BC2CBBBE16DBC125873E002DD576" }
 
     around do |ex|
-      skip "sharded fixture downloads/iec63213 not present" unless File.directory?(SHARDED_FIXTURE)
+      skip "sharded fixture downloads/iec-63213 not present" unless File.directory?(SHARDED_FIXTURE)
       Dir.mktmpdir("cdd-per-version") do |tmp|
         @tmp = tmp
         code_dir = File.join(tmp, "KEA001")
@@ -100,7 +101,7 @@
 
   describe "legacy flat layout (back-compat)" do
     around do |ex|
-      skip "sharded fixture downloads/iec63213 not present" unless File.directory?(SHARDED_FIXTURE)
+      skip "sharded fixture downloads/iec-63213 not present" unless File.directory?(SHARDED_FIXTURE)
       Dir.mktmpdir("cdd-flat") do |tmp|
         @tmp = tmp
         code_dir = File.join(tmp, "KEA001")
@@ -135,4 +136,38 @@ def active_xls_source(code)
       .find { |p| Dir.children(p).any? { |f| f =~ /\Aexport_.*\.xls\z/i } }
     nested || src
   end
+
+  # Classifies each export_*.xls in +fixture_path+ by data row count.
+  # Warns on stderr naming every header-only export type. Pure
+  # visibility nudge — does not fail the suite.
+  def warn_poor_fixture(fixture_path)
+    header_only = {}
+    Dir.glob("#{fixture_path}/*/export_*.xls").each do |xls|
+      type = File.basename(xls)[/export_([A-Z]+)_/, 1]
+      next unless type
+      row_count = count_data_rows(xls)
+      next if row_count.nil? || row_count > 1
+      header_only[type] ||= 0
+      header_only[type] += 1
+    end
+    return if header_only.empty?
+    warn "[opencdd] sharded fixture #{fixture_path} has header-only exports: " \
+         "#{header_only.map { |t, n| "#{t} (#{n} file#{'s' if n > 1})" }.join(', ')}. " \
+         "Re-scrape with `harvest/download.py --dictionary iec-63213` for richer data."
+  end
+
+  # Returns the number of rows in the first sheet of +xls_path+, or
+  # +nil+ if the file cannot be parsed. Header-only files return 1.
+  # Uses the same +spreadsheet+ gem (not Roo) that
+  # +Opencdd::Parcel::WorkbookReader::SpreadsheetSource+ uses for
+  # +.xls+ files in production.
+  def count_data_rows(xls_path)
+    require "spreadsheet"
+    return nil unless [".xls"].include?(File.extname(xls_path).downcase)
+    book = Spreadsheet.open(xls_path)
+    ws = book.worksheet(0)
+    ws.rows.count
+  rescue StandardError
+    nil
+  end
 end

spec/parcel/versioned_reader_spec.rb

@@ -2,17 +2,17 @@
 
 require "spec_helper"
 
-# Exercises the per-version sharded layout in downloads/iec63213.
+# Exercises the per-version sharded layout in downloads/iec-63213.
 # KEA012 has 3 versions (current v003 + two superseded), each in its
 # own UNID subfolder. The fixture is the only multi-version dict
 # available without large downloads.
 RSpec.describe Opencdd::Parcel::VersionedReader do
-  SHARDED_FIXTURE = File.expand_path("../../downloads/iec63213", __dir__)
+  SHARDED_FIXTURE = File.expand_path("../../downloads/iec-63213", __dir__)
 
   let(:reader) { described_class.new(SHARDED_FIXTURE) }
 
   before(:all) do
-    skip "sharded fixture downloads/iec63213 not present" unless File.directory?(SHARDED_FIXTURE)
+    skip "sharded fixture downloads/iec-63213 not present" unless File.directory?(SHARDED_FIXTURE)
   end
 
   describe "#versions_for" do

spec/parcel/writer_smoke_spec.rb

@@ -8,15 +8,15 @@
 # Exercises the same path used by `rake browser:build_parcel[<dict>]`:
 # load_database → Parcel::Writer#write → Parcel::WorkbookReader round-trip.
 RSpec.describe "Parcel build-pipeline smoke" do
-  SOURCE_DIR = File.expand_path("../../downloads/iec63213", __dir__)
+  SOURCE_DIR = File.expand_path("../../downloads/iec-63213", __dir__)
 
   before(:all) do
-    skip "downloads/iec63213 fixture not present" unless File.directory?(SOURCE_DIR)
+    skip "downloads/iec-63213 fixture not present" unless File.directory?(SOURCE_DIR)
   end
 
   let(:database) { Opencdd::Reader.load_database(SOURCE_DIR) }
 
-  it "loads iec63213, emits a Parcel xlsx, and reloads equivalent entities" do
+  it "loads iec-63213, emits a Parcel xlsx, and reloads equivalent entities" do
     dir = Dir.mktmpdir("parcel-smoke")
     out_path = File.join(dir, "IEC63213.xlsx")
     begin

spec/property_id_aliases_spec.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+
+RSpec.describe Opencdd::PropertyIds, "alias registry integrity" do
+  # Regression guard for the alias-collision audit (TODO.complete/1).
+  # The audit (item A6 in data-private/TODO.full-cdd/17) flagged a
+  # hypothetical collision where two REGISTRY entries share an alias
+  # and `alias_map` silently keeps whichever was inserted last. As of
+  # this snapshot, no such collision exists in the REGISTRY. This
+  # spec pins that invariant so a future regression is caught at
+  # spec time, not at runtime.
+
+  describe ".alias_map — no alias is claimed by multiple IDs" do
+    it "does not allow two REGISTRY entries to share an alias" do
+      collisions = {}
+      Opencdd::PropertyIds::REGISTRY.values.each do |entry|
+        entry.aliases.each do |a|
+          collisions[a] ||= []
+          collisions[a] << entry.id unless collisions[a].include?(entry.id)
+        end
+      end
+      shared = collisions.select { |_, ids| ids.size > 1 }
+      expect(shared).to be_empty,
+                        "aliases claimed by multiple REGISTRY IDs: #{shared.inspect}"
+    end
+  end
+
+  describe ".canonical_id — resolves every alias to exactly one ID" do
+    it "resolves 'symbol' to MDC_P025_1 (the IEC 61360 canonical)" do
+      # MDC_P025_1 owns `symbol`, `preferred_symbol_text`, `symbol_in_text`.
+      # If another entry (e.g. MDC_P021 or MDC_P023) ever tries to claim
+      # `symbol`, the collision spec above will catch it first; this
+      # assertion documents the intended resolution.
+      expect(Opencdd::PropertyIds.canonical_id("symbol")).to eq("MDC_P025_1")
+    end
+
+    it "resolves 'unit_symbol' to MDC_P023" do
+      expect(Opencdd::PropertyIds.canonical_id("unit_symbol")).to eq("MDC_P023")
+    end
+
+    it "resolves 'unit_structure' to MDC_P023" do
+      expect(Opencdd::PropertyIds.canonical_id("unit_structure")).to eq("MDC_P023")
+    end
+
+    it "resolves 'definition_class' to MDC_P021" do
+      expect(Opencdd::PropertyIds.canonical_id("definition_class")).to eq("MDC_P021")
+    end
+  end
+
+  describe ".canonical_id — passes canonical IDs through unchanged" do
+    it "returns MDC_P021 unchanged" do
+      expect(Opencdd::PropertyIds.canonical_id("MDC_P021")).to eq("MDC_P021")
+    end
+
+    it "returns MDC_P023 unchanged" do
+      expect(Opencdd::PropertyIds.canonical_id("MDC_P023")).to eq("MDC_P023")
+    end
+
+    it "returns MDC_P025_1 unchanged" do
+      expect(Opencdd::PropertyIds.canonical_id("MDC_P025_1")).to eq("MDC_P025_1")
+    end
+  end
+end