diff --git a/.rubocop.yml b/.rubocop.yml index 578f546..d26f34f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -33,6 +33,7 @@ Metrics/ParameterLists: CountKeywordArgs: false Metrics/ClassLength: + Max: 100 Exclude: # Test classes are mostly tables of cases. - test/**/* diff --git a/CHANGELOG.md b/CHANGELOG.md index f17ceac..45b8581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,76 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `config.validate_languages = false` globally. See [Languages](docs/languages.md). +- **Cache keys change for any document containing a `pre` or `code` + element.** `pre` and `code` are now opaque (see Added, below), so what + gets sent to the provider changed, and what gets keyed changed with it; + an entry cached under the old behaviour keeps serving what the old + behaviour produced. Give the configuration a new `cache_namespace`, or + let `cache_ttl` lapse, to get every such document retranslated. See + [Caching](docs/caching.md#what-a-cache-key-is-made-of). + +- **A runtime `cache_namespace` change now moves the rate limiter too.** + `cache_namespace` names the limiter's own bookkeeping namespace as well + as the cache store's; it used to move only the store, leaving the + limiter counting silently under the old namespace. `active_record_base` + behaves the same way and for the same reason -- the SQL-backed limiter + builds its model from that class just as the store does. See + [Configuration](docs/configuration.md#changing-configuration-at-runtime). + +- **A provider with a blank `cache_key` now raises + `TranslationDiff::InvalidProviderError`, not + `TranslationDiff::Translator::Error`.** The two are siblings under + `TranslationDiff::Error`, not parent and child, so an application + rescuing the old class specifically stops catching this failure. + Rescue `TranslationDiff::Error` to catch both. See + [Errors](docs/errors.md). + ### Added +- **Every event from one `translate` call now shares a `call_id`.** Generated + once per call, opaque, and never derived from the text, it lands in + `translate`, `cache`, `request`, `rate_limit`, `usage` and `cache_error` + alike. Before it, a subscriber receiving `cache` or `request` events had no + way to tell which `translate` call they belonged to, short of tagging + `Thread.current` itself -- a workaround that breaks the moment two + translations share a thread. See + [Instrumentation](docs/instrumentation.md). + +- **`translate` now carries `characters`: the total this call considered, + hit or miss.** A call served entirely from cache never fires a `request` + event and used to report nothing about its size; it now reports a number + there instead. `request`'s own `characters` keeps its narrower meaning -- + what one batch actually sent -- so the two fields share a name but not an + event: summing the wrong one produces a wrong bill. See + [Instrumentation](docs/instrumentation.md). + +- **`TranslationDiff.preview` predicts a `translate` call without making + it.** It answers how many sentences a call would send, how many the cache + already has, and how many characters that is -- without calling a + provider and without writing anything. A preview never pays for language + detection, so `from:` is required wherever there is anything to preview; + leaving it unset raises `TranslationDiff::Previewer::Error`. Built for an + editor that wants to show "this edit will send 1 sentence" before the + author saves. See + [Caching](docs/caching.md#asking-what-a-call-would-do-without-doing-it). + +- **`pre` and `code` are no longer sent for translation, and changing a + configuration option at runtime now rebuilds only what it actually + feeds.** `pre` and `code` join `script` and `style` in + `config.opaque_elements`, the set `TranslationDiff::Passage` never treats + as prose -- default `%i[script style pre code]`, widen or narrow it as + needed -- after a live `
` block came back from Google with
+  `jq '.meters'` mangled into `jq '.metros'`, because nothing told the
+  pipeline that code holds language, not prose. Separately, `provider` and
+  the cache, pool, rate and segmenter options each rebuild only their own
+  collaborator now, instead of nothing: before this, switching `provider`
+  at runtime meant `TranslationDiff.reset!` and reconfiguring from scratch,
+  discarding a Redis pool that had no reason to go. See
+  [Configuration](docs/configuration.md#changing-configuration-at-runtime).
+  Two upgrade consequences of this are filed under Breaking, above:
+  cache keys changing for a document containing `pre` or `code`, and a
+  runtime `cache_namespace` change now moving the rate limiter too.
+
 - A `usage` instrumentation event, firing once per provider request, beside
   `translate`, `cache`, `request` and `rate_limit`. Its payload carries
   `provider`, `characters` (what this library sent, counted locally),
@@ -143,6 +211,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **A `notranslate` span nested inside an opaque element (`pre`, `code`,
+  `script` or `style`) no longer silences every sentence after it.**
+  Closing the protected span used to leave the scanner's own opacity depth
+  one too high, so nothing past it was ever handed to the segmenter again.
+  Found while adding the `pre`/`code` opaque elements above, and fixed the
+  same way for all four. See [How it works](docs/how-it-works.md#html).
 - **Google and DeepL translations in HTML mode no longer come back
   double-escaped.** Both vendors return entity-escaped text -- an
   apostrophe as `'`, a quote as `"`, an ampersand as `&` -- and
diff --git a/README.md b/README.md
index e469e7d..1a85048 100644
--- a/README.md
+++ b/README.md
@@ -67,6 +67,13 @@ end
 formal.translate(contract, from: "en", to: "de", formality: :more)
 ```
 
+```ruby
+# See what a call would send and find cached, without calling the provider or writing anything
+preview = TranslationDiff.preview(contract, from: "en", to: "de")
+preview.sendable_sentences # => sentences not yet cached
+preview.cached_sentences   # => sentences already cached
+```
+
 ```ruby
 # Redis-backed cache, shared across processes
 TranslationDiff.configure { |config| config.redis_url = ENV["REDIS_URL"] }
diff --git a/docs/caching.md b/docs/caching.md
index 5834f64..8b439c7 100644
--- a/docs/caching.md
+++ b/docs/caching.md
@@ -58,6 +58,48 @@ you nothing.
 No options at all contributes no field to the key, which is the four-field
 key every already-warm cache is keyed on.
 
+## Asking what a call would do, without doing it
+
+`TranslationDiff.preview` answers what a `translate` call would send and
+find cached, without calling a provider and without writing anything: how
+many sentences it would send, how many the cache already has, and how many
+characters that is. It reads the same store, through the same
+`SentenceCache`, keyed the same way -- see [What a cache key is made
+of](#what-a-cache-key-is-made-of) above -- so a preview and the call it
+predicts always agree.
+
+```ruby
+preview = TranslationDiff.preview(article_body, from: "en", to: "es")
+preview.sendable_sentences   # => 1, not yet cached
+preview.cached_sentences     # => 4, already cached
+preview.sendable_characters  # => 23
+preview.characters           # => 412, the total this call would consider
+```
+
+`sendable_sentences` and `cached_sentences` are the same two counts the
+`cache` event reports as `misses` and `hits`; `characters` is the same total
+the `translate` event reports. A preview and the call it predicts are
+answering the same question through the same numbers, so "this edit will
+send 23 of 412 characters" and what the events for that call later report
+should agree.
+
+**`from:` is required wherever there is anything to preview.** `translate`
+can leave `from:` unset and pay for one `#detect` request to find it; a
+preview never calls the provider, so it cannot pay for that request either.
+Passing `to:` alone raises `TranslationDiff::Previewer::Error`, naming the
+provider and telling you to pass `from:` explicitly -- unless the document
+holds nothing translatable, or the source and target already match, in
+which case there is nothing to preview and an empty result comes back
+regardless of `from:`.
+
+This is the supported way to ask an editor's question before it becomes a
+bill -- show "this edit will send 1 sentence" before the author saves:
+
+```ruby
+preview = TranslationDiff.preview(edited_body, from: "en", to: "es")
+"This edit will send #{preview.sendable_sentences} sentence#{'s' unless preview.sendable_sentences == 1}."
+```
+
 ## The cache store contract
 
 `config.cache` accepts either a registered name (`:redis`, `:memory`,
diff --git a/docs/configuration.md b/docs/configuration.md
index bca53cf..50db04f 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -69,6 +69,7 @@ at all, so an unset environment variable never has to be special-cased.
 | `rate_interval` | `60` | Seconds over which `rate_limit` (or a limiter's own default threshold) is measured. **Actually enforced over roughly 5-600 seconds** -- see [The rate limiter contract](contracts.md#the-rate-limiter-contract). |
 | `rate_limiter` | `nil` | A registered name (`:redis`, `:active_record`) or an object satisfying the [rate limiter contract](contracts.md#the-rate-limiter-contract). `nil` with `rate_limit` also `nil` means no rate limiting; `nil` with `rate_limit` set resolves to `:redis`. Setting `rate_limiter` alone -- with `rate_limit` left unset -- is enough to turn rate limiting on, at the limiter's own default threshold; it no longer needs `rate_limit` set to avoid crashing. |
 | `segmenter` | `:pragmatic` | The sentence segmenter: a registered name or an object satisfying the [segmenter contract](contracts.md#the-segmenter-contract). |
+| `opaque_elements` | `%i[script style pre code]` | Element names `TranslationDiff::Passage` never treats as prose, whatever they contain, matched case-insensitively so `
` and `")
   end
 
+  # pre and code join script and style: their text is left alone, however much of it looks like prose.
+  def test_pre_and_code_contents_are_not_prose
+    source = %(
See:
curl -s https://example.com/level | jq '.meters'
after
)
+
+    assert_equal ["See:", "after"], cores(source)
+    assert_round_trips(source)
+  end
+
+  # Ox hands back element names exactly as written -- :PRE, :Pre, :STYLE -- never lowercased, so an opaque set
+  # compared case-sensitively misses every one of these and the markup still reaches the provider.
+  def test_uppercase_pre_and_code_contents_are_not_prose
+    source = %(
See:
curl -s https://example.com/level | jq '.meters'
after
)
+
+    assert_equal ["See:", "after"], cores(source)
+    assert_round_trips(source)
+  end
+
+  def test_mixed_case_pre_and_code_contents_are_not_prose
+    source = "
See:
keep me
after
"
+
+    assert_equal ["See:", "after"], cores(source)
+    assert_round_trips(source)
+  end
+
+  def test_uppercase_script_and_style_contents_are_not_prose
+    assert_equal %w[аль бра кил], cores("альбракил")
+  end
+
+  def test_mixed_case_script_and_style_contents_are_not_prose
+    assert_equal %w[аль бра кил], cores("альбракил")
+  end
+
+  # The common case, not the block one: a code span mid-sentence must not split the sentence around it or eat a space.
+  def test_an_inline_code_span_leaves_the_sentence_around_it_intact
+    source = "Press Ctrl+C to stop."
+
+    assert_equal ["Press", "to stop."], cores(source)
+    assert_equal "PRESS Ctrl+C TO STOP.", translated(source)
+  end
+
+  def test_an_empty_code_element_round_trips
+    assert_round_trips("Before.After.")
+  end
+
+  def test_a_code_element_holding_only_whitespace_round_trips
+    assert_round_trips("Before.   After.")
+  end
+
+  def test_a_pre_holding_markup_looking_text_round_trips
+    assert_round_trips("
<div> not real markup
After.")
+  end
+
+  # A notranslate span nested inside an opaque element used to leak: closing it left @opaque_depth one too high,
+  # so every sentence after the code block silently stopped being sent. This is the regression test for that.
+  def test_a_notranslate_span_inside_a_code_block_does_not_confuse_the_walker
+    source = %(DO_NOT_TOUCH After this all good.)
+
+    assert_equal [%(DO_NOT_TOUCH), "After this all good."], cores(source)
+    assert_equal %(DO_NOT_TOUCH AFTER THIS ALL GOOD.), translated(source)
+  end
+
+  # Protection beats opacity on purpose, same as it does for script: a code span inside notranslate stays one unit.
+  def test_a_code_span_inside_a_notranslate_element_stays_inside_the_protected_unit
+    source = %(x After.)
+
+    assert_equal [source], cores(source)
+  end
+
+  # The set is configurable: an application can widen it, or shrink it back to script and style.
+  def test_opaque_elements_is_configurable_per_passage
+    source = "Before.keep meAfter."
+    subject = TranslationDiff::Passage.new(source, segmenter: TranslationDiff::Segmenters::Pragmatic.new,
+                                                   opaque_elements: %i[script style])
+
+    assert_equal ["Before.", "keep me", "After."], subject.segments.reject(&:empty?).map(&:core)
+  end
+
   def test_a_comment_is_not_prose
     assert_equal ["Visible text here."], cores(" Visible text here.")
   end
@@ -73,7 +157,7 @@ def test_a_notranslate_span_reaches_the_provider_with_its_tags
     assert_equal [%(Bold Mountain is a good place.)], cores(source)
   end
 
-  # Ox lowercases element names but not attribute names, and HTML attribute names are case-insensitive.
+  # Ox does not lowercase attribute names either, and HTML attribute names are case-insensitive.
   def test_an_uppercase_class_attribute_still_protects
     source = %(Bold Mountain is a good place.)
 
diff --git a/test/translation_diff/previewer_test.rb b/test/translation_diff/previewer_test.rb
new file mode 100644
index 0000000..c43fa2d
--- /dev/null
+++ b/test/translation_diff/previewer_test.rb
@@ -0,0 +1,223 @@
+require "test_helper"
+
+class PreviewerTest < ConfiguredTest
+  class RecordingProvider < TranslationDiff::Provider
+    def self.capabilities
+      TranslationDiff::Capabilities.new(
+        max_request_size: 1_000, max_batch_size: 10, max_text_size: nil,
+        html: :none, notranslate: false, detects_language: true, reports_billing: false
+      )
+    end
+
+    attr_reader :requests
+
+    def initialize(config)
+      super
+      @requests = []
+    end
+
+    def translate(request)
+      @requests << request
+      TranslationDiff::Translation::Response.build(request: request, texts: request.texts.map(&:upcase))
+    end
+
+    def detect(_text) = "en"
+    def cache_key = "recording"
+  end
+
+  # The capability is the only honest test: every provider inherits a #detect that raises.
+  class BlindProvider < RecordingProvider
+    def self.capabilities
+      TranslationDiff::Capabilities.new(
+        max_request_size: 1_000, max_batch_size: 10, max_text_size: nil,
+        html: :none, notranslate: false, detects_language: false, reports_billing: false
+      )
+    end
+
+    def cache_key = "blind"
+  end
+
+  # An empty cache key would file this provider's translations in every other provider's namespace.
+  class NamelessProvider < RecordingProvider
+    def cache_key = "   "
+  end
+
+  # Counts every write the pipeline attempts, whichever contract it uses, so "writes nothing" has real evidence.
+  class WriteTrackingStore
+    attr_reader :write_calls
+
+    def initialize
+      @inner = TranslationDiff::MemoryCacheStore.new(max_size: 100)
+      @write_calls = 0
+    end
+
+    def read_multi(keys) = @inner.read_multi(keys)
+
+    def write_multi(pairs)
+      @write_calls += 1
+      @inner.write_multi(pairs)
+    end
+  end
+
+  class Recorder
+    attr_reader :events
+
+    def initialize = @events = []
+
+    def instrument(name, payload)
+      @events << [name, payload]
+      yield if block_given?
+    end
+  end
+
+  def setup
+    super
+    @provider = RecordingProvider.new(TranslationDiff::Configuration.new)
+    TranslationDiff.configure { |c| c.cache = :memory }
+  end
+
+  def preview(values, **)
+    TranslationDiff.preview(values, provider: @provider, **)
+  end
+
+  def test_a_document_nothing_has_cached_counts_every_sentence_as_sendable
+    result = preview("One. Two.", from: "en", to: "ru")
+
+    assert_equal 2, result.sendable_sentences
+    assert_equal 0, result.cached_sentences
+    assert_equal "One.Two.".size, result.sendable_characters
+    assert_equal "One.Two.".size, result.characters
+  end
+
+  # The denominator a fully-cached call still needs: sendable_characters alone would report 0 of nothing,
+  # leaving no way to say "this call would send 0 of 8 characters" rather than "there was nothing to send".
+  def test_after_translating_nothing_is_left_to_send
+    TranslationDiff.translate("One. Two.", from: "en", to: "ru", provider: @provider)
+
+    result = preview("One. Two.", from: "en", to: "ru")
+
+    assert_equal 0, result.sendable_sentences
+    assert_equal 2, result.cached_sentences
+    assert_equal 0, result.sendable_characters
+    assert_equal "One.Two.".size, result.characters
+  end
+
+  # This is the property the whole thing exists for: an edit to one sentence sends exactly that sentence.
+  def test_editing_one_sentence_sends_exactly_that_sentence
+    TranslationDiff.translate("One. Two.", from: "en", to: "ru", provider: @provider)
+
+    result = preview("One. Three.", from: "en", to: "ru")
+
+    assert_equal 1, result.sendable_sentences
+    assert_equal 1, result.cached_sentences
+    assert_equal "Three.".size, result.sendable_characters
+  end
+
+  # The strongest test available: it pins the preview to the pipeline, not to an idea of the pipeline.
+  def test_the_previews_counts_equal_what_translate_then_reports_through_the_cache_event
+    TranslationDiff.translate("One. Two.", from: "en", to: "ru", provider: @provider)
+    predicted = preview("One. Three.", from: "en", to: "ru")
+
+    recorder = Recorder.new
+    TranslationDiff.configure { |c| c.instrumenter = recorder }
+    TranslationDiff.translate("One. Three.", from: "en", to: "ru", provider: @provider)
+    payload = recorder.events.find { |name, _| name == "cache.translation_diff" }.last
+
+    assert_equal predicted.sendable_sentences, payload[:misses]
+    assert_equal predicted.cached_sentences, payload[:hits]
+  end
+
+  # The strongest available check that a preview's total and a translate call's own report of its size agree.
+  def test_the_previews_total_characters_equals_what_the_translate_event_reports
+    predicted = preview("One. Two.", from: "en", to: "ru")
+
+    recorder = Recorder.new
+    TranslationDiff.configure { |c| c.instrumenter = recorder }
+    TranslationDiff.translate("One. Two.", from: "en", to: "ru", provider: @provider)
+    payload = recorder.events.find { |name, _| name == "translate.translation_diff" }.last
+
+    assert_equal predicted.characters, payload[:characters]
+  end
+
+  def test_an_explicit_provider_and_the_configured_one_agree
+    TranslationDiff.configure { |c| c.provider = @provider }
+
+    from_config = TranslationDiff.preview("One. Two.", from: "en", to: "ru")
+    explicit = TranslationDiff.preview("One. Two.", from: "en", to: "ru", provider: @provider)
+
+    assert_equal from_config, explicit
+  end
+
+  def test_nothing_is_written
+    tracker = WriteTrackingStore.new
+    TranslationDiff.configure { |c| c.cache = tracker }
+    TranslationDiff.translate("One. Two.", from: "en", to: "ru", provider: @provider)
+    writes_after_translate = tracker.write_calls
+
+    preview("One. Two.", from: "en", to: "ru")
+    preview("One. Three.", from: "en", to: "ru")
+
+    assert_equal writes_after_translate, tracker.write_calls
+  end
+
+  def test_the_same_language_needs_no_provider_and_sends_nothing
+    result = TranslationDiff.preview("One.", from: "en", to: "en", provider: Object.new)
+
+    assert_equal 0, result.sendable_sentences
+  end
+
+  def test_a_value_with_nothing_to_preview_resolves_no_provider
+    result = TranslationDiff.preview("", from: "en", to: "ru", provider: Object.new)
+
+    assert_equal 0, result.sendable_sentences
+  end
+
+  # Detection is a paid request; a preview never makes one, so it says so instead of guessing.
+  def test_a_missing_source_language_is_refused_when_the_provider_would_have_to_detect_it
+    error = assert_raises(TranslationDiff::Previewer::Error) { preview("One.", to: "ru") }
+
+    assert_match(/paid request/, error.message)
+    assert_match(/from:/, error.message)
+  end
+
+  def test_a_provider_that_cannot_detect_says_so_before_it_is_called
+    @provider = BlindProvider.new(TranslationDiff::Configuration.new)
+
+    error = assert_raises(TranslationDiff::Previewer::Error) { preview("One.", to: "ru") }
+
+    assert_match(/cannot detect/, error.message)
+  end
+
+  def test_a_missing_target_language_is_refused_by_name
+    error = assert_raises(ArgumentError) { preview("One.", from: "en") }
+
+    assert_match(/to:/, error.message)
+  end
+
+  def test_per_call_options_change_the_cache_key_the_same_way_translate_does
+    TranslationDiff.translate("One.", from: "en", to: "ru", provider: @provider, formality: :less)
+
+    default_options = preview("One.", from: "en", to: "ru")
+    same_options = preview("One.", from: "en", to: "ru", formality: :less)
+
+    assert_equal 1, default_options.sendable_sentences
+    assert_equal 0, same_options.sendable_sentences
+  end
+
+  # Same guard Translator uses, shared through TranslationDiff::Providers.resolve.
+  def test_a_provider_whose_cache_key_is_blank_is_refused_rather_than_lying_about_the_cache
+    @provider = NamelessProvider.new(TranslationDiff::Configuration.new)
+
+    error = assert_raises(TranslationDiff::InvalidProviderError) { preview("One.", from: "en", to: "ru") }
+
+    assert_match(/must define #cache_key/, error.message)
+  end
+
+  def test_no_preview_error_message_carries_the_text_being_previewed
+    secret = "Zaphod Beeblebrox is president."
+
+    error = assert_raises(TranslationDiff::Previewer::Error) { preview(secret, to: "ru") }
+
+    refute_includes error.message, secret
+  end
+end
diff --git a/test/translation_diff/providers_test.rb b/test/translation_diff/providers_test.rb
index 64e2c27..549136b 100644
--- a/test/translation_diff/providers_test.rb
+++ b/test/translation_diff/providers_test.rb
@@ -76,6 +76,36 @@ def test_null_keeps_its_own_cache_key
     assert_equal "null", TranslationDiff::Providers.build(:null, @config).cache_key
   end
 
+  # The one seam Translator and Previewer both resolve a provider through.
+  def test_resolve_with_nil_returns_the_configured_provider
+    @config.provider = :acme
+
+    assert_instance_of AcmeProvider, TranslationDiff::Providers.resolve(nil, @config)
+  end
+
+  def test_resolve_with_a_name_builds_that_provider
+    assert_instance_of AcmeProvider, TranslationDiff::Providers.resolve(:acme, @config)
+  end
+
+  def test_resolve_with_an_object_uses_it_as_is
+    instance = TranslationDiff::Providers.build(:acme, @config)
+
+    assert_same instance, TranslationDiff::Providers.resolve(instance, @config)
+  end
+
+  # A blank cache_key would file a provider's translations in every other provider's namespace.
+  class BlankCacheKeyProvider < TranslationDiff::Provider
+    def cache_key = "   "
+  end
+
+  def test_resolve_refuses_a_provider_whose_cache_key_is_blank
+    instance = BlankCacheKeyProvider.new(@config)
+
+    error = assert_raises(TranslationDiff::InvalidProviderError) { TranslationDiff::Providers.resolve(instance, @config) }
+
+    assert_match(/must define #cache_key/, error.message)
+  end
+
   # A defensive double `require` and a Rails reload both re-run registration; redeclaring must stay silent.
   def test_registering_the_same_provider_twice_is_not_a_conflict
     TranslationDiff::Providers.register(:acme, AcmeProvider)
diff --git a/test/translation_diff/redis_rate_limiter_test.rb b/test/translation_diff/redis_rate_limiter_test.rb
index 56acc7c..34ed58f 100644
--- a/test/translation_diff/redis_rate_limiter_test.rb
+++ b/test/translation_diff/redis_rate_limiter_test.rb
@@ -150,6 +150,20 @@ def test_build_falls_back_to_the_default_threshold_when_rate_limit_is_unset
     built.check(1)
   end
 
+  # The settings screen that found this: changing cache_namespace must move the limiter, not just the cache.
+  def test_changing_the_cache_namespace_moves_the_limiter_to_the_new_redis_namespace
+    server = FakeRedisServer.new
+    config = TranslationDiff::Configuration.new
+    config.rate_limit = 100
+    config.instance_variable_set(:@redis_pool, FakeConnectionPool.new(server))
+
+    config.rate_limiter_instance.check(10)
+    config.cache_namespace = "tenant-42"
+    config.rate_limiter_instance.check(7)
+
+    assert_equal({ "ratelimit:translation-diff:call" => 10, "ratelimit:tenant-42:call" => 7 }, server.totals)
+  end
+
   private
 
   def limiter(server, **)
diff --git a/test/translation_diff/translator_test.rb b/test/translation_diff/translator_test.rb
index 3c5a445..1c17a84 100644
--- a/test/translation_diff/translator_test.rb
+++ b/test/translation_diff/translator_test.rb
@@ -153,10 +153,11 @@ def test_a_provider_that_cannot_detect_says_so_before_it_is_called
     assert_empty @provider.requests
   end
 
+  # Provider resolution, and this guard with it, now lives in TranslationDiff::Providers.
   def test_a_provider_whose_cache_key_is_blank_is_refused_rather_than_sharing_a_namespace
     @provider = NamelessProvider.new(TranslationDiff::Configuration.new)
 
-    error = assert_raises(TranslationDiff::Translator::Error) { translate("one.", from: "en", to: "ru") }
+    error = assert_raises(TranslationDiff::InvalidProviderError) { translate("one.", from: "en", to: "ru") }
 
     assert_match(/must define #cache_key/, error.message)
   end
@@ -201,6 +202,27 @@ def test_the_translate_event_carries_languages_provider_and_a_count
     assert_equal 2, payload[:values]
   end
 
+  # `request`'s own `characters` is only what one batch sent; this is what the call considered, hit or miss.
+  def test_the_translate_event_carries_the_characters_this_call_considered
+    recorder = instrumented
+    instrumented_translate("Hello there.")
+
+    assert_equal "Hello there.".size, payload_for(recorder, "translate")[:characters]
+  end
+
+  # A call served entirely from cache still knows what it considered, even though no request event ever fires.
+  def test_the_translate_event_reports_characters_even_when_the_call_is_served_entirely_from_cache
+    recorder = instrumented
+    instrumented_translate("Hello there.")
+    instrumented_translate("Hello there.")
+
+    payload = recorder.events.reverse.find { |event| event.first == "translate.translation_diff" }.last
+    request_count = recorder.events.count { |event| event.first == "request.translation_diff" }
+
+    assert_equal "Hello there.".size, payload[:characters]
+    assert_equal 1, request_count
+  end
+
   def test_the_cache_event_carries_hit_and_miss_counts
     recorder = instrumented
     instrumented_translate("Hello there.")
@@ -212,6 +234,17 @@ def test_the_cache_event_carries_hit_and_miss_counts
     assert_equal "recording", payload[:provider]
   end
 
+  # One `Translator`, one identifier, in every event that call emits -- a subscriber's only way to group them.
+  def test_every_event_from_one_call_carries_the_same_call_id
+    recorder = instrumented { |c| c.rate_limiter = FakeRateLimiter.new }
+    instrumented_translate("Hello there.")
+
+    call_ids = recorder.events.map { |_, payload| payload[:call_id] }
+
+    refute_nil call_ids.first
+    assert_equal [call_ids.first] * call_ids.size, call_ids
+  end
+
   # The cache is an optimisation: a translation already paid for at the provider must reach the caller regardless.
   def test_a_failing_cache_write_does_not_lose_a_translation_already_paid_for
     TranslationDiff.configure { |c| c.cache = FailingCacheStore.new }
@@ -229,6 +262,13 @@ def test_a_failing_cache_write_emits_a_cache_error_event_naming_the_provider_and
     assert_equal "TranslatorTest::FailingCacheStore::BoomError", payload[:error]
   end
 
+  def test_a_failing_cache_write_events_call_id_matches_the_translate_events
+    recorder = instrumented { |c| c.cache = FailingCacheStore.new }
+    instrumented_translate("Hello there.")
+
+    assert_equal payload_for(recorder, "translate")[:call_id], payload_for(recorder, "cache_error")[:call_id]
+  end
+
   # The instrumentation payload carries the error's class, never the store's own message, which could quote the row.
   def test_a_failing_cache_writes_event_never_carries_the_text_being_translated
     secret = "Zaphod Beeblebrox is president."