diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 0000000..1454f6e --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +4.0.1 diff --git a/README.md b/README.md index ab223ca..c1e72b3 100644 --- a/README.md +++ b/README.md @@ -240,9 +240,91 @@ MyBrand SpecialTerm ``` +## Dictionary Packs + +SpellKit still bundles no dictionaries. A **pack** is a separate gem that carries the data +and registers itself when it loads, so you choose packs in your Gemfile: + +```ruby +gem "spellkit" +gem "spellkit-general-medical" # drug, condition and gene vocabulary + general English +``` + +```ruby +SpellKit.enable_dictionary(:general_medical, lazy: true) +SpellKit.correct("acetaminphen") # => "acetaminophen" +``` + +A pack ships the tuning that was measured against its own data, so you don't have to +rediscover `edit_distance` and `frequency_threshold` yourself. Override anything: + +```ruby +SpellKit.enable_dictionary(:general_medical, edit_distance: 1) +``` + +SpellKit knows the name of no pack — it provides only the mechanism, so a pack can ship +new terms or new tuning without SpellKit releasing anything. + +### If you load this from a Rails initializer, read this + +**An initializer runs in every process that boots the app** — web, `rails db:migrate`, +`rake`, `console`, sidecars — not just the one that searches. Loading a pack eagerly makes +all of them pay, and a ~200k-term pack at `edit_distance: 2` measures **~2.1 GB resident +and ~5.5s**. That has already OOM-killed a memory-constrained migrate init container in +production. + +Pass `lazy: true`. Registration then costs nothing and the index is built on first real +use, which a migrate or rake process never reaches: + +| | time | RSS | +|---|---|---| +| `enable_dictionary(..., lazy: true)` | 0.000s | +0 MB | +| first `correct()` after that | ~5.5s | +2,076 MB | +| every later call | ~0.0001s | +0 MB | + +**Lazy moves the cost, it doesn't remove it.** For a web server you usually want the +server to pay it rather than the first user, so warm it at worker boot: + +```ruby +# config/puma.rb +on_worker_boot { SpellKit.load_dictionary! } +``` + +`stats` and `healthcheck` deliberately **do not** trigger the load — they report +`{"loaded" => false, "deferred" => true}`. A liveness probe must not be able to build the +index in the very process `lazy` protects. + +```ruby +SpellKit.dictionary_loaded? # false until something forces the load +SpellKit.load_dictionary! # force it now; idempotent, no-op when eager +``` + +Memory scales steeply with `edit_distance`: the same pack is **484 MB at 1** versus +**2.1 GB at 2**, because SymSpell's deletion index grows sharply with distance. If you are +memory-constrained, that knob matters more than lazy loading does. + +### Writing a pack gem + +A pack gem ships its data and one registration call: + +```ruby +# lib/spellkit-my-domain.rb +require "spellkit" + +SpellKit::Packs.register(:my_domain, + dictionary: File.expand_path("../data/dictionary.tsv", __dir__), + protected_path: File.expand_path("../data/protected.txt", __dir__), + defaults: {edit_distance: 1, frequency_threshold: 10.0}, + summary: "What this pack covers") +``` + +Bundler requires it automatically, so `SpellKit.enable_dictionary(:my_domain)` then works. +Registration verifies the files exist, so a packaging mistake fails while the stack still +points at your gem. + ## Dictionary Sources -SpellKit doesn't bundle dictionaries, but works with several sources: +SpellKit works with several raw dictionary sources directly: ### Use the Default Dictionary (Recommended) ```ruby diff --git a/lib/spellkit.rb b/lib/spellkit.rb index 41df6f9..90d0eba 100644 --- a/lib/spellkit.rb +++ b/lib/spellkit.rb @@ -17,6 +17,9 @@ require "spellkit/spellkit" end +require_relative "spellkit/packs" +require_relative "spellkit/lazy_checker" + module SpellKit class Error < StandardError; end class NotLoadedError < Error; end @@ -24,6 +27,10 @@ class FileNotFoundError < Error; end class InvalidArgumentError < Error; end class DownloadError < Error; end + # Raised when a pack name is not registered - almost always a data gem missing from + # the Gemfile, since a pack registers itself when its gem loads. + class UnknownPackError < Error; end + # Default dictionary: SymSpell English 80k frequency dictionary DEFAULT_DICTIONARY_URL = "https://raw.githubusercontent.com/wolfgarbe/SymSpell/master/SymSpell.FrequencyDictionary/en-80k.txt" @@ -98,6 +105,83 @@ def stats def healthcheck default.healthcheck end + + # ---------------------------------------------------------------------------------- + # Dictionary packs + # + # SpellKit still bundles no dictionaries. A pack lives in its own gem, registers + # itself on load (see SpellKit::Packs), and brings the tuning that was measured + # against its own data. These methods are the mechanism only - SpellKit knows the + # name of no pack. + # ---------------------------------------------------------------------------------- + + # Configure the DEFAULT checker from a registered pack, so plain SpellKit.correct + # becomes domain-aware. + # + # SpellKit.enable_dictionary(:general_medical) + # SpellKit.enable_dictionary(:general_medical, lazy: true) # defer the index build + # SpellKit.enable_dictionary(:general_medical, edit_distance: 1) + # SpellKit.enable_dictionary(dictionary: "my.tsv") # your own files + # + # `lazy: true` is strongly recommended from a Rails initializer - see LazyChecker for + # why. Eager stays the default so this is non-breaking. + def enable_dictionary(pack = nil, lazy: false, **options) + # Resolve eagerly even when lazy, so an unregistered pack (usually a data gem + # missing from the Gemfile) fails at boot rather than on a user's first search. + @dictionary_pack = pack.nil? ? nil : Packs.fetch(pack) + + self.default = if lazy + LazyChecker.new(pack, options) + else + load!(**pack_load_options(pack, **options)) + end + end + + # An INDEPENDENT checker, for running more than one pack at once. Does not touch the + # default checker. + def dictionary_checker(pack = nil, lazy: false, **options) + Packs.fetch(pack) unless pack.nil? + return LazyChecker.new(pack, options) if lazy + + Checker.new.load!(**pack_load_options(pack, **options)) + end + + # True once the default checker holds a real index. + def dictionary_loaded? + checker = @default + return false if checker.nil? + return checker.loaded? if checker.is_a?(LazyChecker) + + true + end + + # Force a deferred load now; no-op when already loaded or configured eagerly. + def load_dictionary! + checker = @default + checker.load_now! if checker.is_a?(LazyChecker) + checker + end + + # The pack backing the default checker, or nil when configured from raw files. + attr_reader :dictionary_pack + + private + + # Internal: resolve a pack name (or a bare options hash) into load! keyword arguments. + # Private on purpose - see LazyChecker#resolved_options. + def pack_load_options(pack = nil, **overrides) + if pack.nil? + unless overrides.key?(:dictionary) + raise InvalidArgumentError, + "Pass a registered pack name or a dictionary: of your own. " \ + "Registered packs: #{Packs.names.inspect}" + end + + return overrides + end + + Packs.fetch(pack).load_options(**overrides) + end end end @@ -113,10 +197,9 @@ class SpellKit::Checker alias_method :_rust_healthcheck, :healthcheck def load!(dictionary: nil, protected_path: nil, protected_patterns: [], - edit_distance: 1, frequency_threshold: 10.0, - skip_urls: false, skip_emails: false, skip_hostnames: false, - skip_code_patterns: false, skip_numbers: false, **_options) - + edit_distance: 1, frequency_threshold: 10.0, + skip_urls: false, skip_emails: false, skip_hostnames: false, + skip_code_patterns: false, skip_numbers: false, **_options) # Validate dictionary parameter raise SpellKit::InvalidArgumentError, "dictionary parameter is required" if dictionary.nil? @@ -320,7 +403,7 @@ def download_dictionary(url) raise SpellKit::InvalidArgumentError, "Invalid URL: #{url} (#{e.message})" rescue Timeout::Error => e raise SpellKit::DownloadError, "Download timed out: #{url} (#{e.message})" - rescue StandardError => e + rescue => e raise SpellKit::DownloadError, "Failed to download dictionary: #{e.message}" end @@ -356,13 +439,13 @@ def fetch_with_redirects(url, max_redirects: 5, open_timeout: 10, read_timeout: raise SpellKit::DownloadError, "HTTP #{response.code}: #{response.message} (#{url})" end end - rescue Net::OpenTimeout => e + rescue Net::OpenTimeout raise Timeout::Error, "Connection timeout after #{open_timeout}s: #{url}" - rescue Net::ReadTimeout => e + rescue Net::ReadTimeout raise Timeout::Error, "Read timeout after #{read_timeout}s: #{url}" rescue SocketError => e raise SpellKit::DownloadError, "Network error: #{e.message} (#{url})" rescue OpenSSL::SSL::SSLError => e raise SpellKit::DownloadError, "SSL verification failed: #{e.message} (#{url})" end -end \ No newline at end of file +end diff --git a/lib/spellkit/lazy_checker.rb b/lib/spellkit/lazy_checker.rb new file mode 100644 index 0000000..7450c3a --- /dev/null +++ b/lib/spellkit/lazy_checker.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +module SpellKit + # A checker that has not built its index yet. + # + # WHY. A Rails initializer runs in EVERY process that boots the app - web, db:migrate, + # rake, console, sidecars - but usually only the web server ever spell-checks anything. + # Loading eagerly makes all of them pay, and the cost is not small: a ~200k-term pack at + # edit_distance 2 measures ~2.1 GB resident and several seconds to index. That is not + # hypothetical - it OOM-killed a memory-constrained migrate init container in production. + # + # WHAT DOES NOT TRIGGER A LOAD. `stats` and `healthcheck` deliberately do not, and that + # matters more than it first appears: a liveness probe hitting a health endpoint would + # otherwise build the whole index in exactly the process this class exists to protect, + # silently reintroducing the bug. They report the deferred state instead. + # + # THREAD SAFETY. A threaded server can take two concurrent first requests; without the + # mutex both would build a multi-gigabyte index. The build happens at most once. + class LazyChecker + # Lookups need a real index. Introspection must not build one. + FORCES_LOAD = %i[correct correct? suggestions correct_tokens].freeze + + attr_reader :pack_name + + def initialize(pack_name, options) + @pack_name = pack_name + @options = options + @mutex = Mutex.new + @checker = nil + end + + def loaded? + !@checker.nil? + end + + # Build the index now. Idempotent and thread-safe. + # + # Lazy loading MOVES the cost rather than removing it, so for a web process you + # usually want the server to pay it instead of the first user: + # + # # config/puma.rb + # on_worker_boot { SpellKit.load_dictionary! } + def load_now! + return @checker if @checker + + @mutex.synchronize do + # Re-check inside the lock; another thread may have loaded while we waited. + @checker ||= Checker.new.load!(**resolved_options) + end + end + + FORCES_LOAD.each do |name| + define_method(name) { |*args| load_now!.public_send(name, *args) } + end + + def stats + loaded? ? @checker.stats : deferred_report + end + + def healthcheck + loaded? ? @checker.healthcheck : deferred_report + end + + def method_missing(name, *args, &block) + if Checker.method_defined?(name) + load_now!.public_send(name, *args, &block) + else + super + end + end + + def respond_to_missing?(name, include_private = false) + Checker.method_defined?(name) || super + end + + private + + # Resolved here rather than through a module-level helper so that helper can stay + # private: 1.0.0 freezes the public surface, and an internal seam that only exists to + # let one collaborator reach a private method should not be part of it. + def resolved_options + return @options if @pack_name.nil? + + Packs.fetch(@pack_name).load_options(**@options) + end + + def deferred_report + {"loaded" => false, "deferred" => true, "pack" => @pack_name&.to_s} + end + end +end diff --git a/lib/spellkit/packs.rb b/lib/spellkit/packs.rb new file mode 100644 index 0000000..400496b --- /dev/null +++ b/lib/spellkit/packs.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module SpellKit + # A dictionary pack that some other gem has registered. + # + # SpellKit ships NO packs and knows the name of none. It provides the mechanism; a + # separate data gem (spellkit-general-medical, say) requires itself, calls + # SpellKit::Packs.register, and supplies both the files and the tuning that was measured + # against them. That keeps the "SpellKit doesn't bundle dictionaries" promise in the + # README literally true while still letting `SpellKit.enable_dictionary(:some_pack)` + # work, and it means a pack can ship a new version - new terms, new tuning - without + # SpellKit releasing anything at all. + Pack = Struct.new(:name, :dictionary, :protected_path, :defaults, :summary, keyword_init: true) do + # The keyword arguments to hand to load!, with caller overrides applied last. + def load_options(**overrides) + options = {dictionary: dictionary.to_s} + options[:protected_path] = protected_path.to_s if protected_path + defaults.merge(options).merge(overrides) + end + end + + # The registry of packs some installed gem has announced. + module Packs + class << self + def registry + @registry ||= {} + end + + # Called by a data gem when it loads. + # + # SpellKit::Packs.register(:general_medical, + # dictionary: "#{__dir__}/../data/dictionary.tsv", + # protected_path: "#{__dir__}/../data/protected.txt", + # defaults: {edit_distance: 2, frequency_threshold: 1.0}) + # + # `defaults` is how a pack ships the tuning that was measured against ITS data, + # rather than leaving every consumer to rediscover it. + def register(name, dictionary:, protected_path: nil, defaults: {}, summary: nil) + key = normalize(name) + + # Checked here, at require time, rather than on first use: a data gem whose files + # did not survive packaging is broken, and saying so while the stack still points + # at that gem beats an inexplicable failure during someone's first search. This is + # a stat, so it costs nothing and does not defeat lazy loading, which exists to + # defer the index BUILD. + unless File.exist?(dictionary.to_s) + raise FileNotFoundError, + "Pack #{key.inspect} registered a dictionary that does not exist: #{dictionary}" + end + + if protected_path && !File.exist?(protected_path.to_s) + raise FileNotFoundError, + "Pack #{key.inspect} registered a protected-terms file that does not exist: #{protected_path}" + end + + registry[key] = Pack.new( + name: key, dictionary: dictionary, protected_path: protected_path, + defaults: symbolize(defaults), summary: summary + ) + end + + def registered?(name) + registry.key?(normalize(name)) + end + + # [:general_medical, ...] + def names + registry.keys.map(&:to_sym) + end + + def fetch(name) + registry.fetch(normalize(name)) do + raise UnknownPackError, unknown_message(name) + end + end + + # Test hook. + def reset! + @registry = {} + end + + # :general_medical, "general_medical" and "general-medical" all address one pack. + def normalize(name) + name.to_s.strip.downcase.tr("-", "_") + end + + private + + def unknown_message(name) + if registry.empty? + "No dictionary packs are registered. A pack lives in its own gem - add one to " \ + "your Gemfile (e.g. gem \"spellkit-general-medical\") and it registers itself " \ + "when it loads. To use your own files instead, pass them directly: " \ + "SpellKit.enable_dictionary(dictionary: \"path/to.tsv\")." + else + "Unknown dictionary pack #{name.inspect}. Registered: #{names.inspect}. A pack " \ + "registers itself when its gem loads, so a missing one usually means the gem is " \ + "not in your Gemfile." + end + end + + def symbolize(hash) + (hash || {}).each_with_object({}) { |(key, value), acc| acc[key.to_sym] = value } + end + end + end +end diff --git a/lib/spellkit/version.rb b/lib/spellkit/version.rb index 3ace8a6..7dc7854 100644 --- a/lib/spellkit/version.rb +++ b/lib/spellkit/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module SpellKit - VERSION = "0.3.0" + VERSION = "1.0.0" end diff --git a/spec/lazy_checker_spec.rb b/spec/lazy_checker_spec.rb new file mode 100644 index 0000000..ac43af4 --- /dev/null +++ b/spec/lazy_checker_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +RSpec.describe SpellKit::LazyChecker do + let(:dictionary) { File.expand_path("fixtures/test_unigrams.tsv", __dir__) } + + before do + SpellKit::Packs.reset! + SpellKit::Packs.register(:demo, dictionary: dictionary, defaults: {edit_distance: 1}) + end + + after { + SpellKit::Packs.reset! + SpellKit.default = nil + } + + describe "deferral" do + it "registers without building an index" do + SpellKit.enable_dictionary(:demo, lazy: true) + + expect(SpellKit.dictionary_loaded?).to be false + end + + it "fails at boot on an unregistered pack, not on first search" do + expect { SpellKit.enable_dictionary(:nope, lazy: true) } + .to raise_error(SpellKit::UnknownPackError) + end + end + + describe "what triggers the load" do + before { SpellKit.enable_dictionary(:demo, lazy: true) } + + it "loads on a real lookup" do + expect(SpellKit.correct("helllo")).to eq("hello") + expect(SpellKit.dictionary_loaded?).to be true + end + + it "does NOT load on stats" do + # A liveness probe must not be able to build a multi-gigabyte index in the very + # process lazy loading exists to protect. + expect(SpellKit.stats).to include("deferred" => true, "loaded" => false) + expect(SpellKit.dictionary_loaded?).to be false + end + + it "does NOT load on healthcheck" do + expect(SpellKit.healthcheck).to include("deferred" => true) + expect(SpellKit.dictionary_loaded?).to be false + end + + it "reports real stats after loading" do + SpellKit.correct("helllo") + + expect(SpellKit.stats["loaded"]).to be true + end + end + + describe "explicit warm-up" do + it "loads on demand, for a web-server boot hook" do + SpellKit.enable_dictionary(:demo, lazy: true) + + SpellKit.load_dictionary! + + expect(SpellKit.dictionary_loaded?).to be true + end + + it "is a no-op when the checker was loaded eagerly" do + SpellKit.enable_dictionary(:demo) + + expect { SpellKit.load_dictionary! }.not_to raise_error + expect(SpellKit.dictionary_loaded?).to be true + end + end + + describe "thread safety" do + it "builds the index at most once under concurrent first use" do + # Without the mutex two simultaneous first requests each build an index. Count the + # builds rather than trusting the lock by inspection. + # Count actual index builds rather than a helper call: load! IS the expensive thing, + # so counting it cannot drift if the resolution path is refactored again. + builds = 0 + counter = Mutex.new + allow_any_instance_of(SpellKit::Checker).to receive(:load!).and_wrap_original do |original, *args, **kwargs| + counter.synchronize { builds += 1 } + original.call(*args, **kwargs) + end + + SpellKit.enable_dictionary(:demo, lazy: true) + 8.times.map { Thread.new { SpellKit.correct("helllo") } }.each(&:join) + + expect(builds).to eq(1) + end + end + + describe "backwards compatibility" do + it "loads eagerly by default, so existing callers are unaffected" do + SpellKit.enable_dictionary(:demo) + + expect(SpellKit.dictionary_loaded?).to be true + end + + it "reports not-loaded when nothing is configured" do + SpellKit.default = nil + + expect(SpellKit.dictionary_loaded?).to be false + end + end + + describe "independent lazy checkers" do + it "defers without touching the default checker" do + checker = SpellKit.dictionary_checker(:demo, lazy: true) + + expect(checker.loaded?).to be false + expect(checker.correct("helllo")).to eq("hello") + expect(checker.loaded?).to be true + end + end +end diff --git a/spec/packs_spec.rb b/spec/packs_spec.rb new file mode 100644 index 0000000..074bd2c --- /dev/null +++ b/spec/packs_spec.rb @@ -0,0 +1,110 @@ +# frozen_string_literal: true + +RSpec.describe SpellKit::Packs do + let(:dictionary) { File.expand_path("fixtures/test_unigrams.tsv", __dir__) } + let(:protected_terms) { File.expand_path("fixtures/protected.txt", __dir__) } + + before { described_class.reset! } + after { + described_class.reset! + SpellKit.default = nil + } + + def register(name = :demo, **overrides) + described_class.register(name, dictionary: dictionary, **overrides) + end + + describe "registration" do + it "records a pack a data gem announces" do + register + + expect(described_class.names).to eq([:demo]) + expect(described_class.registered?(:demo)).to be true + end + + it "ships no packs of its own, so SpellKit knows the name of none" do + # The "SpellKit doesn't bundle dictionaries" promise, as a test. + expect(described_class.names).to be_empty + end + + it "treats symbol, string and dashed spellings as one pack" do + register(:general_medical) + + expect(described_class.registered?("general-medical")).to be true + expect(described_class.registered?("General_Medical")).to be true + end + + it "rejects a dictionary file that did not survive packaging" do + # Checked at require time, while the stack still points at the offending gem, + # rather than surfacing during someone's first search. + expect { register(dictionary: "/nonexistent/dictionary.tsv") } + .to raise_error(SpellKit::FileNotFoundError, /does not exist/) + end + + it "rejects a missing protected-terms file too" do + expect { register(protected_path: "/nonexistent/protected.txt") } + .to raise_error(SpellKit::FileNotFoundError, /protected-terms file/) + end + end + + describe "pack-supplied tuning" do + it "carries the defaults measured against that pack's own data" do + register(defaults: {edit_distance: 2, frequency_threshold: 5.0}) + + options = described_class.fetch(:demo).load_options + + expect(options[:edit_distance]).to eq(2) + expect(options[:frequency_threshold]).to eq(5.0) + end + + it "lets a caller override them" do + register(defaults: {edit_distance: 2}) + + expect(described_class.fetch(:demo).load_options(edit_distance: 1)[:edit_distance]).to eq(1) + end + + it "accepts string keys from a data gem and hands load! symbols" do + register(defaults: {"edit_distance" => 2}) + + expect(described_class.fetch(:demo).load_options).to include(edit_distance: 2) + end + end + + describe "unknown packs" do + it "points at the Gemfile when nothing is registered" do + expect { described_class.fetch(:general_medical) } + .to raise_error(SpellKit::UnknownPackError, /add one to your Gemfile/) + end + + it "lists what IS registered when something is" do + register(:demo) + + expect { described_class.fetch(:finance) } + .to raise_error(SpellKit::UnknownPackError, /Registered: \[:demo\]/) + end + end + + describe "SpellKit.enable_dictionary" do + it "loads a registered pack into the default checker" do + register(:demo, protected_path: protected_terms) + + SpellKit.enable_dictionary(:demo) + + expect(SpellKit.dictionary_loaded?).to be true + expect(SpellKit.dictionary_pack.name).to eq("demo") + expect(SpellKit.correct?("hello")).to be true + end + + it "still accepts raw files, with no pack involved" do + SpellKit.enable_dictionary(dictionary: dictionary) + + expect(SpellKit.dictionary_pack).to be_nil + expect(SpellKit.correct?("hello")).to be true + end + + it "asks for a pack or a dictionary when given neither" do + expect { SpellKit.enable_dictionary } + .to raise_error(SpellKit::InvalidArgumentError, /registered pack name or a dictionary/) + end + end +end