9820a0e7c0
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012vJkmBGKKqMoEMzvsXG3y2
139 lines
40 KiB
Markdown
139 lines
40 KiB
Markdown
# SDD ledger — plan: docs/superpowers/plans/2026-10-05-slice-2a-osv-parser.md
|
||
|
||
Branch feat/5-slice-2a-osv-parser, BASE_BRANCH_START b34e113. Spec: openspec/changes/slice-2a-osv-parser/ (reachable). Neighbour worktrees: ../docs, ../fixtures.
|
||
Maркировка: T<n>.<k> task-minor, C/I/M-n final review, R-n rulings (design #m), O-n observations. Status: fixed <commit> | deferred | rejected <why>.
|
||
|
||
## Pre-flight scan (tasks x shared files/interfaces)
|
||
|
||
| Pair / task | Produces vs consumes | Finding |
|
||
|---|---|---|
|
||
| T0 vs T2/T9 | T0: CONTRACT_VERSION 1.1, OsvRow.extra; T2/T9 build OsvDocument/rows with extra | OK |
|
||
| T0 vs spec | schema extra optional null; spec D-9 same | OK |
|
||
| T1 vs T2..T13 | T1: conftest params `osv_file`/`synthetic_file`, fixtures_helpers, mypy strict for src.parser | OK. Note `../fixtures/osv` has 66 real files (+1 stray non-xls?) — helper filters by extension |
|
||
| T2 vs T4/T5/T8 | Sheet/Cell API (`text_at`, `row_cells`, `outline`) consumed by detectors | OK |
|
||
| T2 vs T9 | read_book returns (sheets, warnings); spec D-1/tasks 1.3 say tuple[Sheet,...] | plan is more detailed, not contradictory (spec wins on semantics only) -> OK |
|
||
| T2 test claim | "xls 62 за 3 кв: outline строки 15 (1-based) = 3" unverified | R-10: implementer verifies on real file, adjusts number if wrong (not a contract fact) |
|
||
| T3 vs T4/T5/T8/T9 | Rules classes (OsvHeaderRules.groups etc.) | T5 needs RUB aliases that T3 interface lacks (hardcoded "RUB" would be a literal) -> R-6 |
|
||
| T3 vs T12 linter | Linter bans literals `^[A-Z]{1,3}\d{0,7}$` len<=4 and any literal containing «лист» | CONFLICT: contract enum values (BU, NU, Q1.., RUB, XLS) and error messages ("листов просмотрено N", "лист ОСВ без заголовка", D-6 mandated) trip it -> R-4, R-5 |
|
||
| T4 vs T5 | TableHeader.group_label_rows, text_col, inn_col consumed by build_group_schema | OK |
|
||
| T4 vs T8 | ColumnMap.collapsed (single col in *_debit) vs BodyRow.sums (six values) | GAP: nobody splits the collapsed balance by sign nor emits the spec'd warning (spec scenario "свёрнутое сальдо") -> R-1 |
|
||
| T5 vs T9 | DocHeader -> OsvHeader/OsvGroup; DocHeader.warnings to metrics | OK |
|
||
| T6 vs T8 | BodyRow(is_account, account_rank) is built by classify.collect_body_rows using ACCOUNT_RE (T8) | T6 real-file test (all 66 outline) needs collect_body_rows, which does not exist until T8 -> R-2 |
|
||
| T6 vs T9 | resolve_levels(…, deadline: Deadline); Deadline created in T9 (deadline.py) | CONFLICT (T6 earlier than T9) -> R-3 |
|
||
| T7 vs T6/T8 | to_money used in BodyRow.sums (T8) | T7 after T6 is fine since T6 unit tests pass Decimals directly |
|
||
| T8 vs T9 | ClassifiedRow consumed by checks.py/osv.py; extra, TOTAL | OK |
|
||
| T9 vs T10 | get_parser(settings), app.py passes settings; T10 says "app.py уже" | OK |
|
||
| T9 vs spec scenario "Новый синоним" | spec scenario (add "Нач. остаток" -> parses with no code change) has no test in plan T9/T11/T12 | R-8 |
|
||
| T11 vs T12 | synthetic s01–s13 -> golden snapshots, coverage test may need s14 | OK (plan allows s14) |
|
||
| T11 xlsb | no soffice/LibreOffice on machine (checked) | plan: defer or skip; R-9: allow hand-built BIFF12 writer, else deferred+skip |
|
||
| T12 vs T13 | golden from T12 referenced in docs T13 | OK |
|
||
| each task self-consistency | T0 ok; T1 ok (smoke test ids 66); T2 ok except outline claim (R-10); T3 ok except R-6; T4 ok (needs gridbuilder helper listed); T5 ok except filename-derivation of account code (583 -> 58.03 rule may not fit all 66; R-10 same policy: derive generally, no exception table; mismatch -> ruling); T6 R-2/R-3; T7 ok; T8 R-1; T9 R-3/R-8; T10 ok; T11 R-9; T12 R-4/R-5/R-7; T13 ok |
|
||
|
||
## Rulings (all recorded also in design.md "Decisions made without discussion")
|
||
|
||
- Ruling R-1 (design #11): collapsed (single-column) balance is split by sign in `classify.collect_body_rows` (T8): positive -> debit, negative -> credit (abs), plus one warning "свёрнутое сальдо: opening|closing" per sheet; ColumnMap stays as in T4. — spec scenario requires it, no task owned it — cost if wrong: move split into T9.
|
||
- Ruling R-2 (design #12): T6 delivers unit tests only; the fixtures test `test_hierarchy_real.py` (66 files hierarchy=outline, 66.03 parent) is written in T8 on top of `collect_body_rows`. — BodyRow construction needs ACCOUNT_RE/collect_body_rows from T8 — cost: strategy bugs on real files surface at T8 (opus) rather than T6.
|
||
- Ruling R-3 (design #13): `src/parser/deadline.py` (class Deadline) is created in T6 (needed there and in T8), T9 only reuses it; `Deadline` tests live in T6. — ordering — cost: none.
|
||
- Ruling R-4 (design #14): linter «sheet name» rule = string constant that FULLY matches `(?i)^(лист|sheet)[\s_]*\d*$` (default sheet names), not any literal containing «лист»; messages like «лист ОСВ без заголовка» are allowed. — D-6 mandates such messages; D-10 only forbids sheet names — cost: a sheet-name-like literal inside a longer string is not caught (eyes of final review).
|
||
- Ruling R-5 (design #15): linter exempts (a) constants inside annotations (`Literal[...]`), (b) strings equal to values of contract enums/Literals (`DataType`, `FileType`, quarters, `ErrorCode`…) computed from `src.contract`, not hardcoded; all other short uppercase literals fail. — contract values are structure, not coordinates — cost: a column-letter equal to an enum value (e.g. "BU") slips through.
|
||
- Ruling R-6 (design #16): `data_types.yaml` gets `currency_aliases: {RUB: [руб, рубл, rub]}`; `DataTypeRules.currency_aliases: dict[str, list[Pattern]]`; T5 maps currency via it, no "RUB" literal in code. — avoid literal and keep synonyms as data — cost: dictionary field rename.
|
||
- Ruling R-7 (design #17): the AST linter `test_no_coordinates.py` is created in T3 (sonnet), not T12, so tasks 4–9 are checked as they land; T12 extends/verifies it (tmp-tree negative test, coverage). — a late linter would find violations after 8 tasks — cost: T3 slightly larger.
|
||
- Ruling R-8 (design #18): T9 adds a unit test for spec scenario "Новый синоним": tmp copy of rules dir + «Нач. остаток» synonym, in-memory book with that header parses with no code change; default dictionary MUST NOT contain this synonym (T3 note). — scenario had no test — cost: none.
|
||
- Ruling R-9 (design #19): xlsb (T11): no LibreOffice here; the implementer may write a minimal standalone BIFF12 writer (`tests/synthetic/xlsb_writer.py`, test-only) to build `s13`, verified by pyxlsb round-trip; otherwise `deferred` + skip with reason. A hand-built xlsb is not an Excel-produced file -> O-note. — spec scenario XLSB — cost: weaker evidence of real-Excel xlsb compatibility.
|
||
- Ruling R-10 (design #20): numeric claims in plan about real files that were not verified by the plan author (T2 outline "row 15 = 3"; T5 account code from file name incl. «583 -> 58.03») are verified by the implementer; a wrong claim is fixed in the test by a general rule (never an exception table per file), and noted in the report. — Fixtures are not an oracle — cost: none.
|
||
|
||
## Progress
|
||
Task 0: implemented (docs ec7c42f; here f03bfca..947d2a6), review: T0.1 Important (needs-check example row 3: level/parent inconsistent, data_type NU workaround), T0.2 Minor README v1.0 -> both go to fix round 1
|
||
Task 0: fix round 1/5 dispatched (resumed implementer a56d404d71ee7bfe6)
|
||
Task 0: fix round 1/5 (2 addressed, 0 open; commits 947d2a6..777d4aa, docs ec7c42f..186d6f1)
|
||
Task 0: complete (tabular-parser f03bfca..777d4aa; docs ec7c42f, 186d6f1; review clean)
|
||
Findings: T0.1 fixed 777d4aa/186d6f1 (row 3 became «Вал.» CURRENCY row level 2: honest single violation; reviewer's suggested BU layouts cannot give a single violation — accepted); T0.2 fixed 777d4aa.
|
||
O-1: implementer copied examples/ into api/contracts/examples (identity test covers examples; brief missed it) and EXAMPLES_COUNT=9 — accepted.
|
||
Task 1: complete (commits 777d4aa..38386b5, review clean)
|
||
Task 1: minor T1.1 (deferred): osv_fixture_files не исключает скрытые/lock-файлы (`~$x.xlsx`) — несовместимо с synthetic_files
|
||
Task 1: minor T1.2 (deferred): ARM_KA_FIXTURES без expanduser()
|
||
Task 1: minor T1.3 (deferred): пустой osv/ при существующем корне даёт generic "empty parameter set" skip
|
||
O-2: openspec tasks.md 1.1 называет фикстуру `osv_files`, код/план — `osv_file` (+`synthetic_file`); спека-текст устарел, правится в T13 (tasks.md). Status: deferred (T13).
|
||
Task 2: implemented b5a3b48 (opus implementer a68d0a08ac8cde190), review dispatched. O-3: D-1 в design.md/tasks 1.3 расходятся с брифом по API (row/text/nonempty_rows, bool kind, read_book -> (sheets, warnings)); план главнее по деталям, правка текста — T13. R-10 outline claim verified true.
|
||
Task 2: review: T2.1 Important (whitespace-only cells stored as text) -> fix round 1 dispatched (resumed implementer). Minors: T2.2 (deferred): xlsb BOOLERR hex vs xls "#N/A" mismatch; T2.3 (deferred): fake pyxlsb pads/rows differ from real, real xlsb covered in T11; T2.4 (deferred, pointer for T6): indent = alignment.indent or leading spaces (different units); T2.5 (deferred, pointer for T5/T11): xlsb dates stay numbers (pyxlsb no formats), not in the warning text.
|
||
- Ruling R-11 (design #21): spec-wording drift is reconciled in T13, not now: design D-1 (Sheet API: row/text/nonempty_rows, no bool kind) and tasks.md 1.3 (read_book -> tuple[Sheet,...], «три читателя дают одинаковые ячейки») are updated to the built API `read_book -> (sheets, warnings)`; the three-readers-identical test is dropped (no xls/xlsb writers) — plan is the detailed contract, test coverage is replaced by per-reader tests + real files. Cost if wrong: a reader may later diverge in cell values undetected until golden. [O-3 resolved by R-11]
|
||
Task 2: fix round 1/5 (1 addressed, 0 open — T2.1; commits b5a3b48..c6a768d)
|
||
Task 2: complete (commits 38386b5..c6a768d, review clean)
|
||
Findings: T2.1 fixed c6a768d. T2.2..T2.5 deferred (see above). O-4 (out-of-scope, deferred): xlsb n_rows/n_cols computed from cells keys (trailing whitespace → smaller dims than xlsx/xls); grid.py docstrings mention whitespace cells partly redundant.
|
||
Task 3: implemented fcd609c (sonnet), review dispatched. Notes for later tasks: T4/T5 — "Контрагенты, ИНН" is one cell in real files (has_inn by label text); Match positions refer to normalized text; values after ":" taken separately. T12 — rules coverage needs synthetic for statement_markers, KPP/code/quantity, roman quarters, date ranges, NU/SUM, Вал./Кол.
|
||
Task 3: complete (commits c6a768d..fcd609c, review clean — Approved, no Critical/Important)
|
||
Task 3: minor T3.1 (deferred; final review to triage): linter ADDRESS_MAX_LENGTH=4 misses "AB123"/"E1000" (5 chars)
|
||
Task 3: minor T3.2 (rejected: not a hole; Literal[...] exemption wider than R-5 wording is needed because `Alias = Literal[...]` is not AnnAssign; deviation noted, design #15 covers)
|
||
Task 3: minor T3.3 (deferred -> pointer T8): test_counterparty_named_like_total is vacuous; real risk «Итого-Сервис» matches total_row => T8 needs structural guard (last level-0 row with sums), not text alone
|
||
Task 3: minor T3.4 (deferred): normalize_text lacks NFC/NFKC (decomposed ё/й)
|
||
Task 3: minor T3.5 (deferred): periods «годов» also matches «полугодовой»
|
||
Task 3: minor T3.6 (deferred -> pointer T4): Pattern.search renormalizes text on every call; T4 should normalize a cell once (search on pre-normalized)
|
||
Task 3: minor T3.7 (deferred): loader accepts empty lists / patterns matching the empty string
|
||
Task 3: minor T3.8 (deferred): linter flags literal "sheet"/"лист" alone (dict keys), doesn't catch cell(3,4)/rows[5]/row in (1,2)
|
||
Task 3: minor T3.9 (deferred): test_rules_real hardcodes BU/RUB for every file
|
||
Spec check (⚠️ from reviewer): "падение старта при битом словаре" — load_rules is called by get_parser (T9); make sure T9 wires it (cost: none).
|
||
Task 4: implemented a833945 (opus), review dispatched. Pointers: T8 must skip rows after «Итого» that hold only numbers right of the table (RB 60 Q1 2024); D-2/tasks 2.1 text drift (find_osv_sheets, TableHeader.rows/groups) -> T13 (as R-11). T4 added Pattern.search_normalized (T3.6 resolved), ColumnMap.indexes(), explicit OTHER groups in osv_groups.yaml.
|
||
Task 4: review: T4.1 Important (any 2 of 3 groups accepted; spec allows only «начало» missing; preamble false header) + minors T4.2 (side after punctuation in one-row header), T4.3 (INN leading zero 9/11 digits), T4.4 (no min values for 80% INN rule), T4.6 (vertically-merged group title stretched to next group) -> fix round 1 dispatched (resumed implementer). T4.5 (extra_cols collected over footer after «Итого») deferred -> pointer T8. T9 pointer: `columns` diagnostics must add INN letter. T8 pointer: pad INN 9/11 digits with leading zero.
|
||
- Ruling R-12 (design #22): header group row MUST contain turnover and closing (opening optional); candidate rows lacking them are skipped; else InvalidFormatError naming the missing group(s). — spec allows only «начало» to be absent; brief's "≥2 of 3" is a plan defect — cost if wrong: a file with opening+turnover only (no closing) would error instead of parsing partially.
|
||
Task 4: fix round 1/5 (5 addressed, 0 open — T4.1,T4.2,T4.3,T4.4,T4.6; commits a833945..0015532)
|
||
Task 4: complete (commits fcd609c..0015532, review clean). T4.1 fixed 0015532 (R-12), T4.2 fixed 0015532, T4.3 fixed 0015532, T4.4 fixed 0015532, T4.6 fixed 0015532, T4.5 deferred (-> T8 pointer).
|
||
T4.7 (minor, deferred): numeric KПП column (9 digits) / 11-digit numbers could be mistaken for INN when no «ИНН» header exists (value-based fallback ignores a «КПП» header); mitigation: skip columns whose header matches service_columns["kpp"].
|
||
O-5 pointers: T8 must use detect.inn_from_cell for counterparty_inn; T9 columns diagnostics add INN letter; is_header_repeat keeps ≥2 groups.
|
||
Task 5: implemented 83dd241 (sonnet), review dispatched. Notes: periods.yaml got required `end` key (PeriodRules.end); linter Task 3 adjusted (Literal[1,2] no longer treated as cell subscript); has_inn omitted (not False) when absent; org name containing «за»/«валюта» is treated as a label (limitation).
|
||
Task 5: review: T5.1 Important (org name containing standalone «за»/«валюта» hijacks period/currency), T5.2 Important (periods.end truncates date ranges «г. - ... г.»); minors T5.3 (digit boundary in test regex), T5.4 (variant-2 has_inn untested), T5.5 (xlsb date head test weak), T5.7 (periods.end required key test) -> fix round 1 dispatched; T5.6 (double _subkonto_items) deferred.
|
||
Task 5: fix round 1/5 (6 addressed, 0 open — T5.1,T5.2,T5.3,T5.4,T5.5,T5.7; commits 83dd241..8e627e8)
|
||
Task 5: complete (commits 5868b34..8e627e8, review clean). T5.1 fixed 8e627e8, T5.2 fixed 8e627e8, T5.3 fixed 8e627e8, T5.4 fixed 8e627e8, T5.5 fixed 8e627e8, T5.7 fixed 8e627e8, T5.6 deferred.
|
||
T5.8 (minor, deferred): _find_period picks first recognised candidate in reading order, not the title row (org name containing a year/quarter above the title would still win) — one-line fix: prefer candidate that is also _is_title.
|
||
T5.9 (minor, deferred): currency label without ":" (bare «Валюта» + value in next cell) no longer recognised (deliberate to avoid «Валюта Трейд») → warning "валюта в шапке не найдена".
|
||
Task 6: implemented 1eef422 (opus), review dispatched. Finding by implementer: 2 of 66 real files (schastl 58 and 66, 2 кв 2025) are EMPTY ОСВ (only «Итого», no sums) -> cascade raised InvalidFormatError; implementer added TRIVIAL_STRATEGY="trivial" (<2 rows besides «Итого»).
|
||
- Ruling R-13 (design #23): empty ОСВ (no data rows besides «Итого») is a valid case: hierarchy=`trivial`; «Итого» (TOTAL) is detected as the LAST non-empty row at level 0 whose text matches total_row (structural guard, T3.3) — it does NOT require sums; plan statements "all 66 hierarchy=outline", "metrics.rows > 0 except 66 Q2 2025", "one TOTAL in all 66" are generalized to: outline for every file with data rows; rows==[TOTAL] and metrics.rows==0 for empty ОСВ (property-based, no file list); exactly one TOTAL in all 66. Applies to T8 (TOTAL detection, real test), T9 (property test), T12 (golden). — spec scenario "Пустая ОСВ" already expects rows only TOTAL; the plan assumed one such file, reality has two — cost if wrong: empty ОСВ would be INVALID_FORMAT.
|
||
Task 6: review: T6.1 Important (strategy_sums O(n²), no deadline checks; 4000 rows 7.25 s) + T6.5 minor (deadline tests thin) -> fix round 1 dispatched (resumed implementer a7118b38b9095dee9).
|
||
- Ruling R-14 (design #24): «Итого» is not passed to hierarchy strategies, always level 0 / parent None for all four strategies (T6.2 accepted) — otherwise account_pattern nests it under the last account — cost if wrong: outline/indent accept a sheet whose «Итого» has a non-zero level.
|
||
R-13 text aligned (T6.3): trivial = fewer than two data rows (code), design #23 reworded.
|
||
T6.4 pointer (T8/T13): tasks.md 2.3 fixtures clause («hierarchy=outline у всех 66») reworded to the general rule by T8 (test) / T13 (text).
|
||
T6.6 (minor, deferred): test helper `numbered` needs `# type: ignore[arg-type]`.
|
||
Pointer T8: choose total_row by text AND position BEFORE levels exist (circular: resolve_levels needs total_row; passing None for an unrecognised «Итого» makes account_pattern nest it at level 1) and test that case.
|
||
Task 6: fix round 1/5 (2 addressed, 0 open — T6.1,T6.5; commits 1eef422..68c5e2c)
|
||
Task 6: complete (commits 8e627e8..68c5e2c, review clean). T6.1 fixed 68c5e2c (deadline in strategies; no algorithmic bound — O(n²) worst case remains, interruptible; deferred: algorithmic bound for sums), T6.5 fixed 68c5e2c, T6.2 rejected (ruling R-14), T6.3 fixed (design #23 wording), T6.4 deferred (T8/T13), T6.6 deferred.
|
||
Task 7: implemented 1f2f050 (+60671bc plan tick) (sonnet), review dispatched. Open question from implementer: inn_digits("ИНН 352819549855") fixed as None.
|
||
- Ruling R-15 (design #25): `inn_digits` (and detect.inn_from_cell as owner) extract the digits of a TEXT cell («ИНН 352819549855» → "352819549855"); result only if 10 or 12 digits, else None; numbers keep the leading-zero padding rule (T4.3). — spec: «counterparty_inn — только цифры длиной 10 или 12 из колонки ИНН»; brief test case lists the prefixed text — cost if wrong: counterparty with text prefix gets null INN (matching by name only).
|
||
Task 7: review: T7.1 Important (inn_digits no text extraction; R-15) -> fix round 1 dispatched (design: inn_from_cell stays strict (detection), numbers.inn_digits = strict-then-extract). T7.2 rejected (arbitrary threshold harmless), T7.3 deferred -> open question for T13 (07-open-questions): leading «+», «.5», «5.» rejected in sums. Pointer T8: counterparty_inn via numbers.inn_digits.
|
||
Task 7: fix round 1/5 (1 addressed, 0 open — T7.1; commits 60671bc..46c427e)
|
||
Task 7: complete (commits 5a3d6f9..46c427e, review clean). T7.1 fixed 46c427e (R-15); T7.2 rejected; T7.3 deferred (open question: «+5», «.5», «5.» rejected); T7.4 (minor, deferred -> pointer T8): digit-gluing across non-adjacent groups in inn_digits is looser than needed — tighten to whitespace/NBSP-separated groups (the INN column may be chosen by header alone); T7.5 (minor, deferred): detect.__all__ lacks INN_LENGTHS.
|
||
Task 8: implemented 663ba17 (T7.4 inn_digits tightening) + 4a16f19 (classify) (opus), review dispatched. Real files: 64 outline, 2 trivial, one TOTAL each, max 0.61 s.
|
||
- Ruling R-16 (design #26): ACCOUNT_RE subaccount segments accept 1–3 letters/digits (Cyrillic/Latin), e.g. «76.АВ», «76.ВА», «76.ОТ» are ACCOUNT rows (8 of 66 real files: РБ 76 and schastl 76 for 2024 Q1–Q3); account_rank = number of segments. — spec regex `^\d{2,3}(\.\d{1,2}){0,2}` does not match real data and misclassifies the row as COUNTERPARTY, shifting group_type of children; trees and sums are unaffected — cost if wrong: a counterparty named «76.АВ»-like text (very unlikely) is classified as account. Docs §1.3 and spec regex text updated in T13; golden (T12) must be made after the fix.
|
||
- Ruling R-17 (design #27): counterparty_inn stays per docs §1.3 line 94 and spec — only from the INN column, else null; INN embedded in the row text («Полиграф, 3528315710», 20 rows in schastl 76) is NOT extracted by the parser (counterparty_name = whole row_value); matching by INN-in-name is arm-core's concern -> new open question in docs 07 (T13). The docs example `result-osv-needs-check.json` (row 2 «ООО Лютик, ИНН 7711111111» with counterparty_inn extracted) contradicts the rule -> fix the example in T13 (row_value «ООО Лютик», INN from an INN column). — cost if wrong: arm-core must parse INN from names itself.
|
||
Task 8: review: T8.1 Important (R-16 letter subaccounts, refined regex segment `(?:\d{1,2}|[А-ЯЁA-Z]{1,3})`, uppercase, no IGNORECASE), minors T8.2 (`,\s*` vs spec `,\s` — R-18 keep tolerant, spec text in T13), T8.3 (weak fixtures property: fix), T8.5 (negative assertion), -> fix round 1 dispatched (resumed implementer ad5a18c290747d470). Deferred: T8.4 (spec wording of «Вал.» rows -> T13 docs; R-19), T8.6 (ИНН+КПП glued cell → None) deferred; residual: phone number in INN column accepted (R-15 residual).
|
||
- Ruling R-18 (design #28): `,\s*` kept in ACCOUNT_RE; spec text edited in T13. Cost: «60,Расчеты» treated as account.
|
||
- Ruling R-19 (design #29): «Вал./Кол./currency code» rows: group_type OTHER, data_type CURRENCY/QUANTITY, no schema position, NO inheritance of counterparty_name/inn (account_code_raw inherited); contract example fixed in T13. Cost: consumers find counterparty via parent.
|
||
- Ruling R-20 (design #30): no «Итого» found → warning + checks.total_row_matches_root=false (→ needs_check) in T9. Cost: file without «Итого» always needs_check.
|
||
Pointer T13 (docs): fix example result-osv-needs-check.json: group_schema with ACCOUNT first, group_name of ACCOUNT row = «Счет», row 2 name per R-17 (row_value «ООО Лютик», INN via INN column), row 3 «Вал.» without counterparty_* (R-19); spec/§1.3 regex text (R-16, R-18, R-19); new open question: INN embedded in row text (R-17) and ИНН/КПП glued cell.
|
||
Task 8: fix round 1/5 (4 addressed, 0 open — T8.1,T8.2,T8.3,T8.5; commits 4a16f19..da7215b)
|
||
Task 8: complete (commits 46c427e..da7215b, review clean). T8.1 fixed da7215b (R-16), T8.2 fixed da7215b (R-18: keep `,\s*`, test names tolerance), T8.3 fixed da7215b, T8.5 fixed da7215b, T8.4 deferred (-> T13 docs), T8.6 deferred. Out-of-scope observations (deferred): `\d` in ACCOUNT_RE matches Unicode digits (re.ASCII optional hardening for final review); «76.АВ.АВ» also accepted by {0,2}.
|
||
Task 9: implemented e705d75 (opus), review dispatched. 66/66 done (2 empty ОСВ), max 0.6 s; test_app and integration round-trip moved from NOT_IMPLEMENTED to real-parser errors (UNSUPPORTED_FORMAT/UNREADABLE).
|
||
- Ruling R-21 (design #31): parent-sum check §1.4 — turnovers are compared per column; closing/opening balances are accepted if equal per column, OR if the PARENT shows the balance netted (at most one non-zero side) and the net `debit − credit` equals the children's net. Reason: 1С shows a parent account netted (62 = 16.1M Dt) while subaccounts are shown unnetted (62.01 = 40.2M Dt, 62.02 = 24.1M Kt); 32 of 66 real files fail the strict six-column rule solely for this reason (all 62 offending parents verified: turnovers and net match). Not a fit-to-converge hack: negatives are pinned by tests (wrong turnover, wrong net -> violation). Spec §1.4 text and domain-rules §1.4 are updated in T13. — cost if wrong: an error where parent's net matches but sides are swapped is not caught here (still caught by row identity and TOTAL=root checks).
|
||
Open observation O-6: with the stock dictionary the header «Нач. остаток» is not recognised: result needs_check, opening_* null, opening values go to extra (R-8 test documents it). Pointer T13: CLAUDE.md of repo does not list PARSE_MAX_BYTES / PARSE_TIMEOUT_SECONDS.
|
||
Task 9: complete (commits da7215b..e705d75, review clean — Approved; R-21 implemented tightly). Minors: T9.1 (deferred) add negative tests for R-21: collapsed parent on the opposite side; parent (0,0) with children (X,X); T9.2 (deferred) order of warnings across sheets not asserted; T9.3 (deferred) redundant parentheses in _is_ok; T9.4 (deferred) get_parser() per file in property test.
|
||
Pointers T13 (docs/spec): (a) delta spec tabular-recognition-service must also MODIFY requirement «Результат по контракту» (scenario «Запрос с заглушкой парсера (Срез 0)» in openspec/specs/tabular-recognition-service/spec.md:40 still says NOT_IMPLEMENTED by default) — otherwise archived spec contradicts code; (b) design D-7 says «кэш в get_parser()» — no cache, called once at startup; (c) §1.4/spec text: R-21 (netting at any tree level, residual risks RR-1..4), CLAUDE.md of repo lists PARSE_MAX_BYTES/PARSE_TIMEOUT_SECONDS.
|
||
Task 10: implemented 621c0b2 (sonnet), review dispatched. build_result extracted from worker.py (pure, logic unchanged); CLI exit codes 0/1/2 by status, 3 = file missing/usage (argparse.error overridden); CONCERN: src/__init__ imports app -> Settings(), CLI needs ENV_FILE=.env.test (README documents) — deferred (platform template entry point; follow-up candidate); duration_ms non-deterministic -> T12 golden normalises (already volatile).
|
||
Task 10: complete (commits cf532a3..621c0b2, review clean — Approved). Minors: T10.1 (deferred -> T13 README sentence naming the no-env symptom «ValidationError: OPENBAO Field required → ENV_FILE»; follow-up: lazy app in src/__init__.py (platform template)), T10.2 (deferred) CLI uses get_parser() without settings, size limit applied after read_bytes; T10.3 (deferred) test helpers imported across test modules; T10.4 (deferred) integration result key registered for cleanup after asserts; T10.5 (deferred) `--file-type xls` lowercase → English argparse error (normalise with type=str.upper); T10.6 (info) duration_ms non-deterministic.
|
||
Task 11: implemented (sonnet) fixtures 3b9c16b + here e1559c9; 45 synthetic tests; xlsb via handwritten BIFF12 writer (not Excel-produced: O-7 compat unproven; README has replacement instructions); s09 uses «60.АВ» (not 76.АВ); synthetic_files() now filters to Excel files; generator pins dcterms:modified/zip timestamps. Review dispatched.
|
||
Task 11: complete (fixtures 3b9c16b; here 621c0b2..e1559c9, review clean — Approved). Minors: T11.1 (assigned to T12, fixed there): synthetic tests FAIL (not skip) when fixtures dir exists without osv/synthetic (14 failed) -> skip when synthetic dir absent/empty; T11.2 (assigned to T12): comment in s07 that mixed child signs deliberately exercise R-21 (+ optionally a single-sign collapsed variant); T11.3 (deferred) xlsb writer BrtRowHdr payload may be 16 vs 17 bytes per MS-XLSB, Excel may reject (disclosed: hand-built, Excel compat unproven) — add line to writer docstring; T11.4 (deferred -> fixtures README rewording: reader does not expose outline/format, xlsb supports them); T11.5 (deferred) byte reproducibility across zlib/openpyxl/OS not guaranteed (pin create_system=3); T11.6 (deferred) s10 warning row-number substring match; T11.7 (deferred) coverage synonyms embedded in s-files couple scenarios to dictionary; T11.8 (deferred) generate.py minor cleanups; T11.9 info.
|
||
O-7: xlsb is hand-built (R-9): real Excel-produced xlsb compat unproven — human to replace s13 with an Excel/LibreOffice-converted file (README instruction exists).
|
||
- Ruling R-22 (design #32): rules-coverage test (T12) counts RECOGNITION entries (regex Pattern entries of osv_headers/osv_groups/periods/data_types rows+header+aliases); the `currency_codes` enumeration (ISO 4217 list) is exempt — a code list is not a synonym; entries that never fire on real files/synthetic are either removed from the YAML or covered by a new synthetic file s14_* (allowed, generator + fixtures + test table updated). — spec: «каждая запись словаря SHALL срабатывать»; an ISO list cannot be forced onto fixtures — cost if wrong: dead ISO codes remain.
|
||
Task 12: implemented (sonnet) fixtures fd83a6c (s14–s16) + 8ee574e (82 golden snapshots: 66 real + 16 synthetic), here bc143ef; manual reconciliation of 3 files OK (cells read independently: РБ 60 1кв2025 Итого H677/J677/L677/Q677; schastl 62 xls row 26; 583 → 58.03); R-22 coverage: 4 dictionary entries removed (service_columns.kpp/code/quantity/currency — unused, columns go to extra), 16 covered by s14–s16; linter: ADDRESS_MAX_LENGTH 10 (T3.1), cell(3,4) calls (T3.8); T11.1/T11.2/T3.9 fixed; `make check` ~95 s. Review dispatched.
|
||
Task 12: complete (fixtures fd83a6c, 8ee574e; here afcd7d5..bc143ef, review clean — Approved). Manual reconciliation OK (3 files). Minors: T12.1 (deferred; O-8) s16 variant-2 detail line «…, Валюта» matches header_markers.currency → misleading warning «валюта «» не распознана» frozen in snapshot (currency None either way): a currency label inside the subkonto/detail line should not count as a currency cell — parser defect, low impact; T12.2 (T13) report Decision 5 vs actual: first `make golden-update` prints whole new files as diffs (consider printing only paths for created files); T12.3 (deferred) golden_update deletes orphan snapshots without guard (if osv/synthetic absent → would delete all synthetic snapshots): add guard/--prune; T12.4 (T13 docs) `make check` ≈95 s because `make test` includes the fixtures marker: document in README/CLAUDE.md (or move fixtures to test-fixtures/check-full — controller ruling: document, don't change Makefile); T12.5 (deferred) missing-snapshot failure branch not unit-tested; T12.6 (deferred) golden_helpers imports private cli._error_payload/_file_type; T12.7/T12.8 info.
|
||
Task 13: implemented (sonnet) here 7738221, docs baeb918; make check-full green (1743 passed, integration 9 passed, docker build ok), pytest -m fixtures 808 passed 0 skipped, openspec validate --strict valid in both; added 2 unit tests (new file without snapshot, regression) + check_snapshot moved to golden_helpers (T12.5 fixed); OQ-98…OQ-105 added; example result-osv-needs-check.json got TOTAL row; NOTE: example warnings lack `sheet=<имя>` prefix and organization_inn non-null in example (code writes null) — cosmetic; delta tabular-recognition-service keeps scenario name «Запрос с заглушкой парсера (Срез 0)» (validate --strict) with NullParser-explicit content. Review dispatched.
|
||
Task 13: complete (here bc143ef..7738221, docs baeb918, review clean — Approved). Minors: T13.1 (final fix wave) example result-osv-needs-check.json: warnings should be `sheet=Лист_1 hierarchy=outline` + real Russian violation text from checks.py, organization_inn null, service version — both copies byte-identical; T13.2 (final fix wave) spec/§1.0 «хотя бы одна ячейка» → «в первых 30 строках» (SHEET_MARKER_ROWS); T13.3 (final fix wave) typo in spec.md:122 broken code span; T13.4 (deferred) enumerate residual risks of R-21 (RR-1..4) + (0,0)/(X,X) + «12.ООО» in spec/docs; T13.5 (final fix wave) 06-test-data: «за 2024 г.» → «за 1–3 кв. 2024» (schastl 76); T13.6 (final fix wave) OQ-81 wording: «частично закрыт» in 09-delivery-plan and design.md; T13.7 (final fix wave) task-13-report test name `test_s06_two_osv_sheets_and_other_sheet`; T13.8 (deferred) CLAUDE.md duplicates README module list; T13.9 info (golden_helpers imports pytest).
|
||
Human remainder: after `openspec archive`, openspec/specs/tabular-recognition-service/spec.md:6 Purpose line «В Срезе 0 разбор заменён заглушкой.» stays stale (delta cannot change Purpose).
|
||
FINAL REVIEW (opus, range b34e113..7738221): no Critical. I-1 Important: reading phase unbounded (xlsx with one <mergeCell ref="A1:Z200000"/> 4.9 KB → 24 s, 1.4 GB RSS; PARSE_MAX_BYTES/Deadline don't help; README:188 claims otherwise; no zip expansion ratio cap; defusedxml absent; detect loops without deadline). I-2 Important: unexpected parser exception loses traceback (no logger in src/parser; osv.py:148). M-1 ACCOUNT_RE w/o DOTALL/ASCII (multi-line cell not account; Arabic-Indic digits accepted). M-2 rows after last «Итого» dropped silently. M-3 (=T4.7) numeric КПП column (9 digits padded to 10) can be picked as INN column by value → silent INN=«0»+КПП, status done. M-4 «Итого = первый корень» → needs_check on sheets with several roots (OQ). M-5 hierarchy=sums makes parent-sum check tautological (doc/OQ). M-6 linter blind spots (other names, rows[5], range(5,9), lowercase addresses, TDSheet). M-7 worker downloads whole object before size check. Process: three PRs must merge together (fixtures, docs first, then tabular-parser). Triage: before merge = T13.1/.2/.3/.5/.6/.7 + T9.1 tests + README:188 + I-2 (recommended); rest follow-ups.
|
||
FINAL FIX WAVE (one dispatch, fresh opus): I-1 (partial), I-2, M-1, M-3, T9.1, T1.1, T12.3, T13.1/.2/.3/.5/.6/.7, T13.4 (+ blind spot "both sides +X"), M-4/M-5 as OQs (docs). Deferred: M-2, M-6, M-7, defusedxml (dependency = platform decision), T5.8 etc.
|
||
- Ruling R-23 (design #35): I-1 fixed by (a) cap on total uncompressed size of the xlsx zip (sum ZipInfo.file_size ≤ 10 × PARSE_MAX_BYTES → UNSUPPORTED_FORMAT), (b) pre-scan of `<mergeCell ref>` in xl/worksheets/*.xml rejecting a total merged area above a named constant (UNSUPPORTED_FORMAT with limit text), (c) deadline.check in detect loops (_find_group_row, extra_cols), (d) README claim corrected; defusedxml NOT added (new dependency = platform decision) → follow-up; xls/xlsb: no equivalent merged-range bomb known. — cost if wrong: other bombs (billion laughs) possible until defusedxml/limits are added.
|
||
- Ruling R-24 (design #36): M-3: value-based INN column detection does NOT pad 9/11-digit numbers (padding only when extracting from a column chosen by header or by the strict 80% rule of 10/12 digits); `inn_digits` keeps padding for extraction. — avoids silently taking a numeric КПП column for ИНН; cost: a numeric ИНН column with lost leading zeros and no «ИНН» header is not auto-detected.
|
||
FINAL FIX WAVE done: tabular-parser 9b8b8ea (code: I-1, I-2, M-1, M-3, T9.1, T1.1, T12.3) + 4651513 (docs/spec/example), docs c2005a4; checks: make check 1782 passed, pytest -m fixtures 808 passed, golden-check 82/82 unchanged, integration 9 passed, openspec validate valid both. Limits: MERGED_AREA_LIMIT 200 000 (real max 10 090), ZIP factor 10 (real ratio up to 7.75). Decisions D1 (merged area also checked after reading any format, xls included), D5 (RR-1…RR-4 written by implementer, not defined elsewhere), I-2 tests print stack (pytest --capture=no). Scoped re-review dispatched (opus).
|
||
FINAL FIX WAVE re-review (opus): I-2, M-1, M-3, T9.1, T1.1, T12.3, T13.1–T13.6, OQ-106..109 ADDRESSED; I-1 NOT ADDRESSED in part (b): merge pre-scan bypassable 4 ways (sheet part outside xl/worksheets/; XML charref in ref `Z200000`; namespaced attr before `ref` `a:ref="A1" ref="…"`; `>` inside a quoted attribute) — each restores 24 s / ~1.4 GB from <5 KB file; (a),(c),(d) fine. Residual risks (not covered by R-23, real): cell-volume bomb within zip cap (5 MB xlsx with 2M plain cells → 13 s / 1.5 GB), sharedStrings/styles volume, XML entity expansion (OQ-108). New Minors: N1 after-read `check_merged_area` counts spans > grid._MAX_INDEXED_AREA (10 000) that the grid never expands — a legit whole-column merge in xls now UNSUPPORTED_FORMAT; N2 blind-spot wording «все предки свёрнуты» narrower than the real risk (a collapsed direct parent suffices) in spec.md:182 and 05-domain-rules.md:150.
|
||
- Ruling R-25 (design #37): second (and last) fix round authorized by the human's instruction «C/I — исправлять свежим исполнителем с повторным ревью»: fresh opus implementer replaces the regex pre-scan by a streaming `xml.parsers.expat` scan of ALL `.xml` parts of the zip (namespace-aware: only un-prefixed `ref` on `mergeCell`; charrefs decoded; quoted `>` safe), early abort on area > limit, malformed XML → UNREADABLE; tests for the 4 bypasses + original; N1 (count only spans ≤ the grid indexing threshold in the after-read check) and N2 wording fixed; the cell-volume/shared-strings bomb and entity expansion stay documented residual risks (README «Известные ограничения», OQ-108 extended) — container memory limits/defusedxml are platform decisions. After this round there is no further wave: any residual goes to «Rulings» / follow-up.
|
||
FINAL FIX round 2 done: tabular-parser 6f579b6 (streaming expat scan of ALL zip entries incl. parts that merely look like XML (openpyxl reads xl/data/s.dat — 5th bypass), mergeCell counted in any namespace, ref via openpyxl CellRange, grid.MAX_INDEXED_AREA public, after-read counts only indexable spans, openpyxl_forbidden fixture records calls), docs 68b50a5; checks: test_read_limits 46 passed, make check 1810 passed, fixtures 808 passed, golden-check 82/82, integration 9 passed, openspec valid both; scan cost 12 ms vs 0.58 s (largest real xlsx); 0.8 s vs 10 s (2M cells). Risks: broken *.xml part never read by openpyxl (customXml) → UNREADABLE; billion-laughs protection relies on expat ≥ 2.4 (platform image unverified). Scoped re-review (opus) dispatched.
|
||
FINAL FIX round 2 re-review (opus): N1, N2, README/OQ-108 text ADDRESSED; I-1(b) NOT ADDRESSED: the "looks like XML" sniff (_limits.py:137-149, _SNIFF_BYTES=64, BOM/NUL/whitespace stripped, requires `<`) lets a sheet part named non-`*.xml` (e.g. xl/data/s.dat, Target in workbook.xml.rels) with ≥64 bytes of leading whitespace (or UTF-16 BOM + whitespace), no XML declaration, evade the scan: 1837-byte file → read_book 27.75 s / 1.35 GB (repro /tmp/rr2/N7.xlsx; v.py+h.py). Suggested minimal fix: drop the sniff, feed every non-directory non-*.xml entry to expat in lenient mode (binary fails on the first bytes) + tests. Held: the 4 original bypasses, .dat, DOCTYPE entity/ATTLIST default, sheet-qualified/$-absolute refs, straddling tag, deep nesting, entity bombs (expat 2.6.3 amplification limit, 0.05 s), ref fuzz. New Minors: #2 N1 opens a CPU vector — xls with 100 000 spans of area 10 001: merged_span scans `_large_merged` linearly, 4.4 ms/lookup → cut only by the 120 s Deadline (fix: cap on the number of large spans ≤1000 or interval index); #3 scan memory is not bounded by chunk size: junk part with a 400 MB attribute → 1.3 GB RSS/1.24 s, 400 MB of `<a/>` → 8.5 s CPU in the scan alone, parts openpyxl never reads; older expat (<2.6.0) quadratic on huge tokens; README claim "every part whose start looks like XML is scanned" contradicts the bypass; OQ-105 still says «под свёрнутыми предками» (docs 07-open-questions.md:113).
|
||
- Ruling R-26 (design #38): NO third fix wave (skill: one final wave + one re-review; R-25 already stretched it once on the human's instruction «C/I — исправлять свежим исполнителем с повторным ревью»): the pre-scan is defence-in-depth, NOT a security boundary — it blocks the generic/accidental merge bomb and all standard-form bypasses found, but a deliberately hand-crafted file (non-`.xml` sheet part + ≥64 bytes leading whitespace, no XML declaration) still reaches openpyxl (27 s / 1.4 GB); the real boundary must be a container memory limit and/or subprocess with RLIMIT (platform decision, OQ-108). Docs-only correction dispatched (README claim, OQ-108/OQ-105, new minors listed). TOP FOLLOW-UP for the human: «drop the sniff; scan every non-directory entry leniently with expat» (≈10 lines + test) and the large-span count cap. — cost if wrong: a crafted file takes a worker for ~30 s / 1.4 GB until the follow-up or a platform memory limit; files come from authenticated users via the internal IS, not anonymous.
|