Skip to content

extract_numbers.py: normalize_number matches units by substring, so 150bps becomes 150,000,000,000 #342

Description

@AB2006-personal

Summary

normalize_number in skills/ib-check-deck/scripts/extract_numbers.py matches
unit suffixes by substring containment rather than equality:

for unit_key in sorted(multipliers.keys(), key=len, reverse=True):
    if unit_key.lower() in unit.lower():        # <-- containment
        return base_value * multipliers[unit_key]

'B' is a key worth 1e9, and 'b' is a substring of 'bps'. So every
basis-point figure is multiplied by a billion
.

Reproduce

>>> from extract_numbers import normalize_number
>>> normalize_number('150', 'bps')
150000000000.0          # expected 150
>>> normalize_number('150', 'x')
150.0                   # correct
>>> normalize_number('150', '%')
150.0                   # correct

End to end:

>>> [(n.value, n.normalized) for n in extract_numbers("Spread widened 150bps.")]
[('150bps', 150000000000.0)]

bps is in the pattern's own unit alternation, so this is a unit the extractor
is explicitly built to recognise. Every rate, spread and margin-delta figure in
a deck is inflated by 1e9, and each one then enters find_inconsistencies as a
revenue/margin-category value competing with real figures.

Cause and suggested fix

Containment was presumably chosen so USD_million matches million. But it
makes every single-letter key a substring trap: 'B'→bps is the live one, and
the same shape would bite any future unit containing t, b, m or k.

Match the unit exactly, after normalising case, and keep a separate explicit
mapping for the composite forms the caller builds:

MULTIPLIERS = {
    'trillion': 1e12, 't': 1e12, 'tn': 1e12,
    'billion': 1e9,  'b': 1e9,  'bn': 1e9,
    'million': 1e6,  'm': 1e6,  'mm': 1e6, 'mn': 1e6,
    'thousand': 1e3, 'k': 1e3,
    '%': 1, 'bps': 1, 'x': 1,          # dimensionless — explicit, not by omission
}
key = unit.lower().removeprefix('usd_')
return base_value * MULTIPLIERS.get(key, 1)

Listing %, bps and x as explicit 1× entries is worth doing on its own:
today they return the base value only because nothing matched, which is the
same code path as an unrecognised unit.

Note

Found while verifying the fix for #338 — the negative half of that test
("genuine suffixes must still parse") is what surfaced it. Filing separately
since it is a distinct defect, but I have a fix ready and can include it in
that PR or a separate one, whichever you prefer.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions