fix: tolerate spreadsheet decimal roundoff
This commit is contained in:
@@ -27,6 +27,11 @@ immutable OSS adapter, while download routing retains controlled-local compatibi
|
|||||||
identities while preserving historical local rows. Local fake-object-store, builder, publisher, download-router and
|
identities while preserving historical local rows. Local fake-object-store, builder, publisher, download-router and
|
||||||
migration tests pass; the development machine has no Docker, so CentOS image/Compose acceptance is explicitly handed
|
migration tests pass; the development machine has no Docker, so CentOS image/Compose acceptance is explicitly handed
|
||||||
to the operator. No live report or business data was changed.
|
to the operator. No live report or business data was changed.
|
||||||
|
- Fixed the reported `MONTHLY_REPORT_OUTPUT_VALIDATION_FAILED` false negative for the 2026-08-03 440-row monthly
|
||||||
|
workbook. `_validate_workbook` now scans read-only sheets sequentially with `iter_rows` and compares Excel-decimal
|
||||||
|
readback values within `0.000001`; sheet order, headers, dimensions, semantic identity, formula count and exact
|
||||||
|
`=R[row]*C[row]*G[row]` formulas remain strict. A 440-row/five-sheet decimal regression and the 18-case monthly
|
||||||
|
builder/publishing/worker/service/repository suite pass. The dead/pending outbox events were not modified.
|
||||||
|
|
||||||
## Completed On 2026-08-03
|
## Completed On 2026-08-03
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,7 @@
|
|||||||
|
|
||||||
| Date | Task | Outcome | Docs Updated |
|
| Date | Task | Outcome | Docs Updated |
|
||||||
|---|---|---|---|
|
|---|---|---|---|
|
||||||
|
| 2026-08-04 | Fix false monthly XLSX output-validation failure on 440-row reports | Replaced read-only workbook random-cell validation with sequential `iter_rows` scanning and changed Excel-decimal readback checks to an explicit `0.000001` tolerance. Sheet/header/dimension checks, semantic SHA-256, formula count and exact `=R[row]*C[row]*G[row]` formulas remain strict. Added a 440-row, three-date, five-sheet decimal regression; the XLSX test and 18-case monthly builder/publishing/worker/service/repository suite pass. No outbox, database or live runtime state was changed | Current state, report-artifact evidence, scoped planning record |
|
||||||
| 2026-08-04 | Remove private monthly XLSX runtime and make report artifacts deployment-safe | Replaced monthly and company report builders' production path with Python/openpyxl; preserved the exact monthly `TOTAL PRICE` formula and workbook semantic checks; published new monthly/company XLSX and `result.json` artifacts through the existing OSS adapter while retaining legacy local reads; added migration 016 for OSS monthly artifacts, removed Node builders/flags, and added OSS/local download routing tests. Targeted report/deployment tests pass; the full local suite has 307 passes, 3 skips and 8 environment-only errors (`httpx`/Aliyun test setup). Docker and live OSS were intentionally not run in this development environment | Current state, architecture, deployment runbook, migration ledger, evidence/index, stale item |
|
| 2026-08-04 | Remove private monthly XLSX runtime and make report artifacts deployment-safe | Replaced monthly and company report builders' production path with Python/openpyxl; preserved the exact monthly `TOTAL PRICE` formula and workbook semantic checks; published new monthly/company XLSX and `result.json` artifacts through the existing OSS adapter while retaining legacy local reads; added migration 016 for OSS monthly artifacts, removed Node builders/flags, and added OSS/local download routing tests. Targeted report/deployment tests pass; the full local suite has 307 passes, 3 skips and 8 environment-only errors (`httpx`/Aliyun test setup). Docker and live OSS were intentionally not run in this development environment | Current state, architecture, deployment runbook, migration ledger, evidence/index, stale item |
|
||||||
| 2026-08-03 | Make company-report XLSX generation deploy without private npm | Replaced the company-report builder's Node/private `@oai/artifact-tool` runtime dependency with Python/openpyxl workbook generation and self-validation, removed the private package dependency, enabled `--enable-company-reports` in Compose Web, and documented that only the monthly worker still needs Node/artifact-tool packaging. Focused company-report and deployment-entrypoint tests pass 19/19; no migration or live deployment was performed | Current state/history, architecture, deployment evidence/index, stale item, runbooks |
|
| 2026-08-03 | Make company-report XLSX generation deploy without private npm | Replaced the company-report builder's Node/private `@oai/artifact-tool` runtime dependency with Python/openpyxl workbook generation and self-validation, removed the private package dependency, enabled `--enable-company-reports` in Compose Web, and documented that only the monthly worker still needs Node/artifact-tool packaging. Focused company-report and deployment-entrypoint tests pass 19/19; no migration or live deployment was performed | Current state/history, architecture, deployment evidence/index, stale item, runbooks |
|
||||||
| 2026-08-03 | Execute controlled fix and live-accept 2026-08 `01-10` company report | With explicit confirmation, stopped only the stale PID 11176 and started new listener PID 54127 through the existing Keychain-backed launcher. One authorized job `05cc547d…` succeeded 5/5 with row counts `54/18/7/1/15`, captured active Booking batch-7 source metadata, reused the existing version/artifact identities, and passed five HTTP download/hash checks. Logout and temporary-file cleanup completed; no Booking/Finance source or fact mutation occurred | Current state, runtime evidence/index, stale item, commitments, scoped planning record |
|
| 2026-08-03 | Execute controlled fix and live-accept 2026-08 `01-10` company report | With explicit confirmation, stopped only the stale PID 11176 and started new listener PID 54127 through the existing Keychain-backed launcher. One authorized job `05cc547d…` succeeded 5/5 with row counts `54/18/7/1/15`, captured active Booking batch-7 source metadata, reused the existing version/artifact identities, and passed five HTTP download/hash checks. Logout and temporary-file cleanup completed; no Booking/Finance source or fact mutation occurred | Current state, runtime evidence/index, stale item, commitments, scoped planning record |
|
||||||
|
|||||||
@@ -27,6 +27,20 @@
|
|||||||
- The Docker root dependency chain now installs `openpyxl==3.1.5` through `requirements-monthly-reports.txt`; no
|
- The Docker root dependency chain now installs `openpyxl==3.1.5` through `requirements-monthly-reports.txt`; no
|
||||||
separate npm or Codex-runtime bootstrap is needed.
|
separate npm or Codex-runtime bootstrap is needed.
|
||||||
|
|
||||||
|
## Follow-up: 2026-08-03 monthly validation failure
|
||||||
|
|
||||||
|
- Operator-provided production evidence showed a 440-row, multi-sheet 2026-08-03 workbook whose sheet names and
|
||||||
|
channel counts matched, and which passed manual validation when reopened non-read-only with numeric tolerance. The
|
||||||
|
worker nevertheless emitted `MONTHLY_REPORT_OUTPUT_VALIDATION_FAILED`; outbox event 31 became `dead` and event 30
|
||||||
|
remained pending/retrying. This isolates the failure to the workbook-validation implementation rather than
|
||||||
|
container, worker startup, database or OSS publication.
|
||||||
|
- `_validate_workbook` no longer calls `worksheet.cell(row, column)` on a `read_only=True` workbook. It consumes each
|
||||||
|
sheet header and data row in order with `iter_rows`, rejects missing/extra rows or columns, and keeps strict formula
|
||||||
|
and semantic checks. Excel-decimal values now use a finite-Decimal absolute tolerance of `0.000001`.
|
||||||
|
- Regression coverage builds 440 rows across all five standard sheets over three daily versions, with decimal rate and
|
||||||
|
price values plus KB rows. The builder validates 440 formulas successfully; the dedicated XLSX class passes 2/2 and
|
||||||
|
the monthly builder/publishing/worker/service/repository slice passes 18/18.
|
||||||
|
|
||||||
## Verification
|
## Verification
|
||||||
|
|
||||||
- Targeted report/web/storage/migration suite: 41 tests passed; builder/company integration slice: 15 tests passed
|
- Targeted report/web/storage/migration suite: 41 tests passed; builder/company integration slice: 15 tests passed
|
||||||
|
|||||||
@@ -90,6 +90,7 @@ class OpenpyxlWorkbookBuilder:
|
|||||||
|
|
||||||
_INTEGER_HEADERS = frozenset({"NIGHTS", "ADULTS", "CHILDREN", "NO_OF_ROOMS"})
|
_INTEGER_HEADERS = frozenset({"NIGHTS", "ADULTS", "CHILDREN", "NO_OF_ROOMS"})
|
||||||
_DECIMAL_HEADERS = frozenset({"RATE_AMOUNT", "REAL PRICE", "TOTAL PRICE", KB_HEADER})
|
_DECIMAL_HEADERS = frozenset({"RATE_AMOUNT", "REAL PRICE", "TOTAL PRICE", KB_HEADER})
|
||||||
|
_DECIMAL_TOLERANCE = Decimal("0.000001")
|
||||||
|
|
||||||
def __init__(self, *_legacy_args: Any, **_legacy_options: Any) -> None:
|
def __init__(self, *_legacy_args: Any, **_legacy_options: Any) -> None:
|
||||||
# The ignored arguments keep older local wrappers import-compatible while
|
# The ignored arguments keep older local wrappers import-compatible while
|
||||||
@@ -130,6 +131,17 @@ class OpenpyxlWorkbookBuilder:
|
|||||||
return float(Decimal(str(value)))
|
return float(Decimal(str(value)))
|
||||||
return cls._safe_text(value)
|
return cls._safe_text(value)
|
||||||
|
|
||||||
|
@classmethod
|
||||||
|
def _decimal_matches(cls, actual: Any, expected: Any) -> bool:
|
||||||
|
try:
|
||||||
|
actual_decimal = Decimal(str(actual))
|
||||||
|
expected_decimal = Decimal(str(expected))
|
||||||
|
except (InvalidOperation, TypeError, ValueError):
|
||||||
|
return False
|
||||||
|
if not actual_decimal.is_finite() or not expected_decimal.is_finite():
|
||||||
|
return False
|
||||||
|
return abs(actual_decimal - expected_decimal) <= cls._DECIMAL_TOLERANCE
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _style_sheet(worksheet: Any, headers: list[str]) -> None:
|
def _style_sheet(worksheet: Any, headers: list[str]) -> None:
|
||||||
worksheet.freeze_panes = "A2"
|
worksheet.freeze_panes = "A2"
|
||||||
@@ -185,14 +197,20 @@ class OpenpyxlWorkbookBuilder:
|
|||||||
for channel, expected_row_count in zip(report.channels, expected_rows):
|
for channel, expected_row_count in zip(report.channels, expected_rows):
|
||||||
worksheet = workbook[channel.worksheet]
|
worksheet = workbook[channel.worksheet]
|
||||||
headers = list(channel.headers)
|
headers = list(channel.headers)
|
||||||
if worksheet.max_row != expected_row_count + 1 or worksheet.max_column != len(headers):
|
row_iterator = worksheet.iter_rows(values_only=False)
|
||||||
raise ValueError("worksheet dimensions do not match")
|
header_cells = next(row_iterator, None)
|
||||||
if [worksheet.cell(1, index).value for index in range(1, len(headers) + 1)] != headers:
|
if (
|
||||||
|
header_cells is None
|
||||||
|
or len(header_cells) != len(headers)
|
||||||
|
or [cell.value for cell in header_cells] != headers
|
||||||
|
):
|
||||||
raise ValueError("worksheet headers do not match")
|
raise ValueError("worksheet headers do not match")
|
||||||
for row_index, expected_row in enumerate(channel.rows, start=2):
|
for row_index, expected_row in enumerate(channel.rows, start=2):
|
||||||
|
cells = next(row_iterator, None)
|
||||||
|
if cells is None or len(cells) != len(headers):
|
||||||
|
raise ValueError("worksheet dimensions do not match")
|
||||||
payload_row = expected_row.to_payload(channel.worksheet == "DY-AI-Easy-KB")
|
payload_row = expected_row.to_payload(channel.worksheet == "DY-AI-Easy-KB")
|
||||||
for column, header in enumerate(headers, start=1):
|
for header, cell in zip(headers, cells):
|
||||||
cell = worksheet.cell(row_index, column)
|
|
||||||
if header == "TOTAL PRICE":
|
if header == "TOTAL PRICE":
|
||||||
expected_formula = f"=R{row_index}*C{row_index}*G{row_index}"
|
expected_formula = f"=R{row_index}*C{row_index}*G{row_index}"
|
||||||
if cell.value != expected_formula or cell.data_type != "f":
|
if cell.value != expected_formula or cell.data_type != "f":
|
||||||
@@ -206,11 +224,13 @@ class OpenpyxlWorkbookBuilder:
|
|||||||
elif header in cls._INTEGER_HEADERS:
|
elif header in cls._INTEGER_HEADERS:
|
||||||
matches = actual == int(expected)
|
matches = actual == int(expected)
|
||||||
elif header in cls._DECIMAL_HEADERS:
|
elif header in cls._DECIMAL_HEADERS:
|
||||||
matches = Decimal(str(actual)) == Decimal(str(expected))
|
matches = cls._decimal_matches(actual, expected)
|
||||||
else:
|
else:
|
||||||
matches = ("" if actual is None else actual) == cls._safe_text(expected)
|
matches = ("" if actual is None else actual) == cls._safe_text(expected)
|
||||||
if not matches:
|
if not matches:
|
||||||
raise ValueError("worksheet values do not match")
|
raise ValueError("worksheet values do not match")
|
||||||
|
if next(row_iterator, None) is not None:
|
||||||
|
raise ValueError("worksheet dimensions do not match")
|
||||||
if formula_count != report.row_count:
|
if formula_count != report.row_count:
|
||||||
raise ValueError("formula count does not match")
|
raise ValueError("formula count does not match")
|
||||||
if cls._semantic_sha256(payload) != cls._semantic_sha256(report.to_workbook_payload()):
|
if cls._semantic_sha256(payload) != cls._semantic_sha256(report.to_workbook_payload()):
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import tempfile
|
import tempfile
|
||||||
import unittest
|
import unittest
|
||||||
from datetime import date
|
from datetime import date, timedelta
|
||||||
from decimal import Decimal
|
from decimal import Decimal
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
@@ -13,6 +13,7 @@ from monthly_reports.contracts import (
|
|||||||
DailyVersionPin,
|
DailyVersionPin,
|
||||||
MonthlyFact,
|
MonthlyFact,
|
||||||
MonthlySnapshot,
|
MonthlySnapshot,
|
||||||
|
STANDARD_SHEETS,
|
||||||
)
|
)
|
||||||
from monthly_reports.core import build_monthly_report
|
from monthly_reports.core import build_monthly_report
|
||||||
from monthly_reports.publishing import OpenpyxlWorkbookBuilder
|
from monthly_reports.publishing import OpenpyxlWorkbookBuilder
|
||||||
@@ -57,6 +58,77 @@ def report_fixture():
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def large_decimal_report_fixture():
|
||||||
|
facts = []
|
||||||
|
daily_versions = []
|
||||||
|
channel_observations = []
|
||||||
|
record_id = 1000
|
||||||
|
daily_plan = (
|
||||||
|
(date(2026, 8, 1), 501, 28),
|
||||||
|
(date(2026, 8, 2), 502, 30),
|
||||||
|
(date(2026, 8, 3), 503, 30),
|
||||||
|
)
|
||||||
|
for business_date, daily_version_id, rows_per_sheet in daily_plan:
|
||||||
|
daily_versions.append(DailyVersionPin(business_date, daily_version_id))
|
||||||
|
for channel_index, channel_key in enumerate(STANDARD_SHEETS, start=1):
|
||||||
|
channel_observations.append(
|
||||||
|
ChannelObservation(
|
||||||
|
business_date,
|
||||||
|
daily_version_id,
|
||||||
|
channel_key,
|
||||||
|
channel_index,
|
||||||
|
rows_per_sheet,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
for row_index in range(rows_per_sheet):
|
||||||
|
nights = 1 + (row_index % 3)
|
||||||
|
no_of_rooms = 1 + (row_index % 2)
|
||||||
|
real_price = Decimal("321.17") + Decimal(row_index % 19) / Decimal("100")
|
||||||
|
sequence = record_id
|
||||||
|
facts.append(
|
||||||
|
MonthlyFact(
|
||||||
|
daily_record_id=sequence,
|
||||||
|
daily_version_id=daily_version_id,
|
||||||
|
business_date=business_date,
|
||||||
|
channel_key=channel_key,
|
||||||
|
arrival=business_date,
|
||||||
|
departure=business_date + timedelta(days=nights),
|
||||||
|
nights=nights,
|
||||||
|
adults=2,
|
||||||
|
children=row_index % 2,
|
||||||
|
block_code=f"BLOCK-{sequence}",
|
||||||
|
no_of_rooms=no_of_rooms,
|
||||||
|
company_name=f"COMPANY-{channel_index}",
|
||||||
|
confirmation_no=f"CONF-{sequence}",
|
||||||
|
disp_room_no=f"ROOM-{sequence}",
|
||||||
|
effective_rate_amount=Decimal("247.35")
|
||||||
|
+ Decimal((row_index + channel_index) % 7) / Decimal("100"),
|
||||||
|
full_name=f"GUEST-{sequence}",
|
||||||
|
res_comment=f"COMMENT-{sequence}",
|
||||||
|
trace_text="TRACE",
|
||||||
|
products="ROOM",
|
||||||
|
rate_code="BAR",
|
||||||
|
room_category_label="STD",
|
||||||
|
real_price=real_price,
|
||||||
|
total_price=real_price * no_of_rooms * nights,
|
||||||
|
kb_amount=Decimal(no_of_rooms * 100)
|
||||||
|
if channel_key == "DY-AI-Easy-KB"
|
||||||
|
else None,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
record_id += 1
|
||||||
|
return build_monthly_report(
|
||||||
|
2026,
|
||||||
|
8,
|
||||||
|
date(2026, 8, 3),
|
||||||
|
MonthlySnapshot(
|
||||||
|
tuple(facts),
|
||||||
|
tuple(daily_versions),
|
||||||
|
tuple(channel_observations),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class MonthlyReportsXlsxTests(unittest.TestCase):
|
class MonthlyReportsXlsxTests(unittest.TestCase):
|
||||||
def test_openpyxl_builder_reopens_checks_formulas_and_semantic_identity(self):
|
def test_openpyxl_builder_reopens_checks_formulas_and_semantic_identity(self):
|
||||||
report = report_fixture()
|
report = report_fixture()
|
||||||
@@ -88,6 +160,31 @@ class MonthlyReportsXlsxTests(unittest.TestCase):
|
|||||||
finally:
|
finally:
|
||||||
workbook.close()
|
workbook.close()
|
||||||
|
|
||||||
|
def test_openpyxl_builder_validates_440_decimal_rows_across_multiple_sheets(self):
|
||||||
|
report = large_decimal_report_fixture()
|
||||||
|
self.assertEqual(report.row_count, 440)
|
||||||
|
|
||||||
|
with tempfile.TemporaryDirectory(prefix="monthly-report-xlsx-large-") as temp_dir:
|
||||||
|
built = OpenpyxlWorkbookBuilder().build(report, Path(temp_dir))
|
||||||
|
|
||||||
|
self.assertEqual(built.summary["sheet_names"], list(STANDARD_SHEETS))
|
||||||
|
self.assertEqual(built.summary["row_counts"], [88, 88, 88, 88, 88])
|
||||||
|
self.assertEqual(built.summary["formula_count"], 440)
|
||||||
|
self.assertTrue(
|
||||||
|
OpenpyxlWorkbookBuilder._decimal_matches("321.1700005", "321.17")
|
||||||
|
)
|
||||||
|
self.assertFalse(
|
||||||
|
OpenpyxlWorkbookBuilder._decimal_matches("321.1700011", "321.17")
|
||||||
|
)
|
||||||
|
self.assertEqual(
|
||||||
|
OpenpyxlWorkbookBuilder._validate_workbook(
|
||||||
|
built.path,
|
||||||
|
report,
|
||||||
|
report.to_workbook_payload(),
|
||||||
|
),
|
||||||
|
440,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user