fix: workaround ISTAT endPeriod+1 bug with post-query filter#8
Conversation
…eriod+1 bug ISTAT SDMX API returns one extra year beyond the requested endPeriod. Instead of adjusting the API parameter (which would break if ISTAT fixes the bug), filter rows client-side after fetching. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a client-side workaround in get_data to mitigate ISTAT SDMX’s documented behavior of returning data for one extra year beyond the requested endPeriod, by filtering the parsed TSV output on TIME_PERIOD.
Changes:
- Introduces
filter_tsv_by_time_period()to remove TSV rows outside the requested time range (currently year-based). - Applies the TIME_PERIOD filter after the existing client-side dimension filtering in
handle_get_data. - Logs how many rows were removed by the workaround.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Extract year as int for comparison | ||
| def extract_year(period_str: str) -> int | None: | ||
| try: | ||
| return int(period_str.split('-')[0][:4]) | ||
| except (ValueError, IndexError): | ||
| return None | ||
|
|
||
| start_year = extract_year(start_period) if start_period else None | ||
| end_year = extract_year(end_period) if end_period else None |
There was a problem hiding this comment.
filter_tsv_by_time_period claims to filter rows outside the requested [start_period, end_period] range, but the implementation only compares the extracted year (YYYY) and ignores month/day within the same year. This can return out-of-range rows when callers provide more specific periods (e.g., end_period='2024-03-31'), and it may also silently skip filtering if start_period/end_period aren’t parseable into a year. Consider either (a) parsing and comparing full TIME_PERIODs for the supported formats (YYYY, YYYY-MM, YYYY-MM-DD, etc.) or (b) explicitly documenting/renaming this as a year-only filter and logging a warning when the period can’t be parsed.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Done in f817dc2. Replaced the year-only extraction with a _parse_period helper that normalises all supported formats — YYYY, YYYY-MM, YYYY-MM-DD, YYYY-Qn (quarterly), and YYYY-Sn/YYYY-Hn (semester) — into (year, start_month, end_month) tuples. The filter now correctly excludes rows that fall outside the requested window at sub-year granularity (e.g. end_period='2024-03-31' removes Q2–Q4 rows). When a period string can't be parsed a warning is logged and that filter bound is skipped; rows with unparseable TIME_PERIOD values are kept. 35 unit tests covering all formats and edge cases were also added.
| def filter_tsv_by_time_period(tsv_data: str, start_period: str | None, end_period: str | None) -> str: | ||
| """Filter TSV rows by TIME_PERIOD range (workaround for ISTAT endPeriod+1 bug). | ||
|
|
||
| The ISTAT SDMX API returns one extra year beyond the requested endPeriod. | ||
| This function filters out rows whose TIME_PERIOD falls outside the requested range. | ||
|
|
||
| Args: | ||
| tsv_data: TSV string with header row | ||
| start_period: Requested start period (e.g. '2024' or '2024-01-01') | ||
| end_period: Requested end period (e.g. '2024' or '2024-12-31') | ||
|
|
||
| Returns: | ||
| Filtered TSV string | ||
| """ | ||
| if not start_period and not end_period: | ||
| return tsv_data | ||
|
|
||
| lines = tsv_data.split('\n') | ||
| if not lines: | ||
| return tsv_data | ||
|
|
||
| header = lines[0].split('\t') | ||
| if 'TIME_PERIOD' not in header: | ||
| return tsv_data | ||
|
|
||
| tp_idx = header.index('TIME_PERIOD') | ||
|
|
||
| # Extract year as int for comparison | ||
| def extract_year(period_str: str) -> int | None: | ||
| try: | ||
| return int(period_str.split('-')[0][:4]) | ||
| except (ValueError, IndexError): | ||
| return None | ||
|
|
||
| start_year = extract_year(start_period) if start_period else None | ||
| end_year = extract_year(end_period) if end_period else None | ||
|
|
||
| filtered = [lines[0]] | ||
| removed = 0 | ||
| for line in lines[1:]: | ||
| if not line: | ||
| continue | ||
| parts = line.split('\t') | ||
| if len(parts) <= tp_idx: | ||
| filtered.append(line) | ||
| continue | ||
| row_year = extract_year(parts[tp_idx]) | ||
| if row_year is None: | ||
| filtered.append(line) | ||
| continue | ||
| if start_year and row_year < start_year: | ||
| removed += 1 | ||
| continue | ||
| if end_year and row_year > end_year: | ||
| removed += 1 | ||
| continue | ||
| filtered.append(line) | ||
|
|
||
| if removed: | ||
| logger.info(f'filter_tsv_by_time_period: removed {removed} rows outside [{start_period}, {end_period}]') | ||
|
|
||
| return '\n'.join(filtered) |
There was a problem hiding this comment.
This new client-side TIME_PERIOD filtering logic doesn’t appear to have tests. Adding unit tests for filter_tsv_by_time_period (e.g., verifying 2024 rows are removed when end_period is 2023, and that the header/other rows are preserved) would help prevent regressions and ensure the ISTAT endPeriod+1 workaround behaves as intended.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Already addressed in f817dc2 — tests/test_get_data.py adds 35 unit tests covering filter_tsv_by_time_period and the underlying _parse_period helper. This includes verifying that 2024 rows are removed when end_period is 2023, that the header is always preserved, and cases for all supported period formats (YYYY, YYYY-MM, YYYY-MM-DD, YYYY-Qn, YYYY-Sn/Hn), plus edge cases for unparseable periods and short rows.
…ods; add tests Agent-Logs-Url: https://github.com/ondata/istat_mcp_server/sessions/eb73fae6-cdca-4e31-a287-20f5ee8b860a Co-authored-by: aborruso <30607+aborruso@users.noreply.github.com>
Summary
endPeriodrichiesto (documentato qui)TIME_PERIODche rimuove le righe fuori dal range richiestoYYYY,YYYY-MM,YYYY-MM-DD,YYYY-Qn(trimestrale),YYYY-Sn/YYYY-Hn(semestrale)TIME_PERIODnon parseable vengono mantenuteTest plan
get_dataconend_period="2023-12-31"non restituisca dati 2024end_period="2024-Q2"escluda le righe Q3 e Q4 dello stesso annostart_period/end_periodil comportamento rimanga invariato_parse_periodefilter_tsv_by_time_period(35 test)🤖 Generated with Claude Code
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.