fix(nodes): preserve financial scale headers in the llamaparse node#1594
fix(nodes): preserve financial scale headers in the llamaparse node#1594Ansh-Karnwal wants to merge 4 commits into
Conversation
Financial statements declare magnitude in a caption above the table
("(In millions)", "$ in thousands"). When that caption is separated from
its table by downstream chunking or an LLM step, every figure reads as
raw units and is silently 1,000x / 1,000,000x / 1,000,000,000x off.
Nothing downstream catches it, since a wrong-by-1e6 number looks exactly
like a correct one.
This makes the loss non-silent. A new pure-Python module (scale.py) runs
after the parser assembles its Markdown and:
- detects scale captions the parser already emitted and normalizes each
to a unit, factor, and optional currency. Prose uses of the same words
("millions of users", "billions served", "hundreds of thousands of
dollars", "$5 million in revenue") are rejected.
- welds a normalized marker directly above each table whose caption is in
scope ("> Scale: amounts in millions (x1,000,000)"), so a later chunker
cannot split the scale away from its figures.
- flags any numeric table with no caption in scope with a visible warning
line, and surfaces detected_scales and scale_warnings in
parsing_metadata.
Scope is bounded by page/section breaks so a caption on one page is not
attached to an unrelated table on the next. Annotation is idempotent and
wrapped so any failure falls back to the untouched text and never blocks
parsing.
The table-detection heuristic is factored into scale.py and reused by
IInstance.extract_tables_from_text so there is one implementation, with
no change to the table lane output.
Recovering a caption the parser dropped from the source entirely is out
of scope; that needs source re-extraction and belongs to the audit-grade
datalab_parse node. Documented in the node README.
Adds test_llamaparse_scale.py: 18 labeled positive captions all detected
with the correct factor, 10 adversarial negatives with zero false
positives, plus annotation, scoping, idempotence, and table-parity cases.
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesScale-header preservation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Parser.parse
participant scale.detect_scale_declarations
participant scale.annotate_scale
participant parsing_metadata
Parser.parse->>scale.detect_scale_declarations: detect declarations in text_content
Parser.parse->>scale.annotate_scale: annotate tables with declarations
scale.annotate_scale-->>Parser.parse: annotated text and warning records
Parser.parse->>parsing_metadata: store detected_scales and scale_warnings
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/llamaparse/scale.py`:
- Around line 366-370: Remove the unused table_spans parameter from the
annotate_scale function signature and update all callers to stop passing it,
while preserving the existing declarations and return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 83a48e34-0ccf-4f21-b2d0-82634d6d3a5a
📒 Files selected for processing (5)
nodes/src/nodes/llamaparse/IInstance.pynodes/src/nodes/llamaparse/README.mdnodes/src/nodes/llamaparse/parser.pynodes/src/nodes/llamaparse/scale.pynodes/test/test_llamaparse_scale.py
annotate_scale computes table blocks internally via iter_table_blocks; the table_spans parameter was never read and no caller passed it. The standalone find_markdown_table_spans helper is unchanged.
|
Hi @Ansh-Karnwal — thanks for this PR, it's genuinely nice work. The problem statement is compelling and well-argued, and the engineering shows care: the pure-Python module with no engine dependency, the double failure isolation so parsing can never break, the adversarial negative corpus in the tests, and factoring the table heuristic into one shared implementation (I verified it's behavior-identical to the old one, and the table lane output stays byte-identical). The README section is a model for how node docs should read. I ran the module against a range of documents and found a few things worth addressing before merge, roughly in order of importance: 1. The feature is unconditional, and non-financial documents pay for it. Any numeric table with no caption in scope gets the missing-scale warning — I ran a server-metrics report, an inventory list, and sports standings through it, and all three got "Scale not detected... Figures may be in thousands, millions, or billions" injected into the text lane. For RAG pipelines over ordinary documents that's misleading hedging welded into the content. Suggest either a config flag in 2. Prose false positives weld a wrong scale. "The market opportunity, valued in billions, keeps growing" followed by an unrelated table gets 3. 4. The marker detects itself. 5. Smaller items: the warning marker's emoji should be plain text (project convention is no emoji in application output, and it already tripped a cp1252 None of this diminishes the core idea — welding scale to tables so chunkers can't separate them is the right call, and the execution is close. Happy to discuss any of these. |
Address maintainer review on the scale-header preservation:
- Gate the missing-scale warning on a financial signal. A numeric table
with no caption is warned only when the document has a caption elsewhere
or the table itself carries a currency marking. Plain numeric tables
(inventory, server metrics, standings) are left untouched, so ordinary
documents no longer get hedging welded into the text lane.
- Only weld a caption onto a table that looks numeric, so a caption over a
prose or roster table is never mis-attached.
- Veto prose lead-ins ("valued in billions") so a sentence near a table no
longer welds a wrong scale.
- Stop the injected markers from being read back as captions on a second
pass, keeping re-annotation idempotent across multiple tables.
- Make the markers plain ASCII so they survive a cp1252 console.
- Move the shared-table import to module scope in IInstance and document
the already_annotated status in the README and docstring.
|
Thanks for the thorough read, @Rod-Christensen. All five are addressed in 32a4ca9. 1. Unconditional warning on non-financial docs. The missing-scale warning is now gated on a financial signal: a numeric table is only warned when the document has a caption detected elsewhere, or the table itself carries a currency marking ( 2. Prose false positives welding a wrong scale. Two changes. The weld is now gated on 3. 4. Marker self-detection. 5. Smaller items. Markers are now plain ASCII ( Tests are at 17 (added 5 for the scenarios above), ruff clean. Happy to keep iterating. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/llamaparse/scale.py`:
- Around line 399-400: Define a module-level compiled regex, such as
_CURRENCY_WORDS_RE, near _CURRENCY_WORDS using escaped alternation for all
currency terms, then update _table_has_currency to call its search method on
lowered table text instead of compiling and checking one pattern per word.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fc6ef5d3-6ec1-42bc-abcb-5b730051305a
📒 Files selected for processing (4)
nodes/src/nodes/llamaparse/IInstance.pynodes/src/nodes/llamaparse/README.mdnodes/src/nodes/llamaparse/scale.pynodes/test/test_llamaparse_scale.py
Replace the per-word re.search loop in _table_has_currency with a single module-level compiled alternation (_CURRENCY_WORDS_RE), matching the pattern used elsewhere in the module.
2 participants
Problem
Fixes #1407. Financial statements declare magnitude in a caption above the table ("(In millions)", "$ in thousands"). When that caption gets separated from its table by downstream chunking or an LLM step, every figure reads as raw units and is silently 1,000x / 1,000,000x / 1,000,000,000x off. Nothing downstream catches it, because a wrong-by-1e6 number looks exactly like a correct one.
A repo-wide grep for
scale/millionsinnodes/returns nothing today, so there is no scale handling anywhere in this node.Approach
Make the loss non-silent. A new pure-Python module
scale.pyruns after the parser assembles its Markdown and:> Scale: amounts in millions (×1,000,000). A later chunker or LLM cannot separate the scale from its figures.detected_scalesandscale_warningsinparsing_metadataso downstream nodes get a structured signal, not just prose.Scope is bounded by page/section breaks, so a caption on one page is not attached to an unrelated table on the next. Annotation is idempotent and wrapped so any failure falls back to the untouched text and never blocks parsing.
The
|-row table-detection heuristic is factored intoscale.pyand reused byIInstance.extract_tables_from_text, so there is one implementation with no change to thetablelane output.Out of scope
Recovering a caption that LlamaParse dropped from the source entirely is not handled here. That needs an independent source-text extraction and belongs to the audit-grade
datalab_parsenode (this node's README already documents it as not audit-grade). A hard XBRL / prior-year magnitude cross-check is the natural follow-up.Testing
nodes/test/test_llamaparse_scale.py(pure logic, no LlamaParse API or engine needed):Locally: 12/12 tests pass,
ruff checkandruff format --checkclean on all touched files.Docs
Updated the co-located node README with a "Scale-header preservation" section (detected captions, the injected marker and warning, the new
parsing_metadatakeys, and the out-of-scope note).Summary by CodeRabbit