Skip to content

[T3371] FIX: don't queue B2S letters when the language detector has no opinion (v18) - #2161

Open
NoeBerdoz wants to merge 3 commits into
18.0from
T3371-b2s-letters-wrongly-queued-v18
Open

NoeBerdoz wants to merge 3 commits into
18.0from
T3371-b2s-letters-wrongly-queued-v18

Conversation

@NoeBerdoz

@NoeBerdoz NoeBerdoz commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

[T3371] FIX: don't queue B2S letters when the language detector has no opinion

v18 port of the 14.0 fix. Companion PR: #2160 (14.0). Merge the two
together
, so the two lines don't drift apart again.

Problem

Letters that need no translation are landing on the translation platform — reported on
14.0 for TH / UG / PH / GH / KE / TG / BF, including letters queued to be "translated"
from a language into that same language.

v18 has the same defect: T3339 was ported here in ba4ba7e6, and this is that change's
over-correction.

Root cause

T3339 fixed a real bug: field offices were labelling letters as already-translated when
they were not, Odoo trusted the label, and untranslated letters reached sponsors. It
fixed that by making content detection the deciding input.

The problem is that detect_language() returns an empty recordset for four different
reasons:

  • text shorter than the length floor
  • a language outside en/de/fr/it/es
  • confidence below the threshold
  • an exception

process_letter() could not tell them apart and read all four as "the sponsor cannot
read this → translate"
. Only the second justifies that.

The short-text case dominates. Most B2S letters below the floor are photo captions and
gift acknowledgements that the field office already writes in the sponsor's language —
there is nothing to translate, and no amount of detection will ever say so, because
there is not enough text to detect.

The fix

_letter_language_verdict() reports whether we have an opinion at all.
_sponsor_can_read_letter() then decides:

  • enough text to judge → decide from the content (T3339 behaviour, unchanged)
  • too short to judge → fall back to the field-office language, as before T3339

Both gates call the shared predicate, so they cannot drift apart.

Does this reopen T3339?

No. Letters long enough to judge are still decided by their content — including letters
whose label claims the sponsor can read them but whose content disagrees. That is
exactly the population T3339 was about, and the fallback never touches it. Before
T3339, every letter was decided by the label; after this change, only letters too
short to detect are.

Residual gap: a letter that is both untranslated and below the length floor falls
back to the label, which could be wrong. Detection cannot close this — there is nothing
to detect.

⚠️ Auto-send — needs a deploy decision

_can_auto_send() took the same change as the gate in T3339, and it decides whether a
letter is emailed to the sponsor without human validation. T3339 sharply reduced
automatic sending as an untracked side effect; this change restores it.

Coordinate the deploy with whoever runs the communication queue.

Fixing only the translation gate would release short letters from the translation queue
and strand them at the auto-send check — moving the backlog rather than clearing it.
That is why both gates are in scope.

Changes

Identical to the 14.0 patch — _detect_letter_language() and _can_auto_send() were
byte-identical between the two branches before this change, so this is a straight copy,
not a rewrite.

file change
advanced_translation/models/langdetect.py expose the length floor as min_length instead of a magic number
sbc_compassion/models/correspondence.py add _letter_language_verdict() and _sponsor_can_read_letter(); extract _clean_letter_text(); drop the now-unused _detect_letter_language()
sbc_translation/models/correspondence.py process_letter() calls the shared predicate
partner_communication_compassion/models/correspondence.py _can_auto_send() calls the same predicate
sbc_compassion/tests/test_letter_language_verdict.py new — pins the three verdict outcomes

Testing

3 tests in sbc_compassion/tests/test_letter_language_verdict.py. They pass on 14.0;
not yet executed on 18.0 — the v18 database is the pre-populated migration data and
running the suite against it was out of scope for this port.

_sponsor_can_read_letter() itself has no unit coverage. It needs a persisted letter,
because supporter_languages_ids is related to partner_id.spoken_lang_ids and comes
back NewId-wrapped on an in-memory record. Building one requires BaseSponsorshipTest,
whose setUpClass is broken on 14.0 for every test class that uses it — worth checking
whether the same holds here.

On 14.0 the behaviour was validated by replaying the pre-T3339, current and new
predicates over the full B2S corpus on a database copy, and by checking the gate against
an independent reimplementation through odoo shell. Since the code is identical, that
evidence carries over.

Known issues (pre-existing, not introduced here)

  • The gate is non-deterministic. langdetect is used without setting
    DetectorFactory.seed, so a letter near the confidence threshold can be queued on one
    run and published on the next.
  • Long-but-ambiguous letters are queued even when the label says the sponsor can read
    them, because below-threshold counts as an opinion. Deliberate — it is the safe
    direction for T3339.
  • The confidence threshold is largely inert: probabilities are renormalised after
    filtering to the five languages, so a single surviving candidate always scores 1.0.

Out of scope

  • search_iso639() resolves duplicates by id orderilike + limit=1 with no
    _order on the model, so it always returns the lowest id, and Spanish exists three
    times. Measured impact on this ticket: none.
  • No-op queue entries (source language equal to target): a guard there would be a
    label-trusting bypass into the publish path.

…o opinion

- detect_language() returns empty both for "text too short to judge" and "not a language we handle"; the gate treated both as "translate". Fall back to the field-office language only in the first case, so short captions stop reaching the platform while T3339 stays fixed.
- same correction in _can_auto_send(), which ba4ba7e changed alongside the gate.
- port of the 14.0 fix; the two files were byte-identical beforehand, so this is a straight copy.
@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

Safe to merge.

Reviews (3) · Last reviewed commit: "[T3371] STYLE: apply ruff-format to the ..."

Comment thread partner_communication_compassion/models/correspondence.py
- langdetect samples randomly and runs unseeded; the English test text misses the confidence threshold in ~0.1% of runs, which would fail CI without a product regression. Pin the seed in setUp and restore it on cleanup.
- pre-commit runs ruff-format as well as ruff check; only the latter was run locally.
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.

2 participants