Skip to content

fix: rename progressPct to historicalSyncProgressPct in sync-progress API#90

Open
lgahdl wants to merge 2 commits into
developfrom
luizhatem/fix-progresspct-rename-historicalSyncProgressPct
Open

fix: rename progressPct to historicalSyncProgressPct in sync-progress API#90
lgahdl wants to merge 2 commits into
developfrom
luizhatem/fix-progresspct-rename-historicalSyncProgressPct

Conversation

@lgahdl
Copy link
Copy Markdown
Contributor

@lgahdl lgahdl commented Jun 2, 2026

Summary

  • Rename progressPcthistoricalSyncProgressPct in the GET /api/sync-progress response schema, endpoint implementation, route description, and test assertions

Motivation

The old name was ambiguous — progressPct: 100 while isComplete: false looked contradictory. The new name makes it self-documenting: it tracks historical block-fetch progress (eth_getLogs layer), not handler execution completion.

Test plan

  • pnpm typecheck passes (verified locally)
  • pnpm test passes — all 31 tests green (verified locally)

🤖 Generated with Claude Code

… API

Makes the field name self-documenting — it clearly tracks historical
block-fetch progress (not handler completion), consistent with the
distinction between progressPct=100 and isComplete=false.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lgahdl
Copy link
Copy Markdown
Contributor Author

lgahdl commented Jun 2, 2026

Documentation / CI review

docs/api-reference.md — example response and description still use progressPct. Must fix before merge.

Lines 56, 63, and 70 of docs/api-reference.md were not updated:

  • Line 56: "progressPct": 42.9, in the example JSON block
  • Line 63: "progressPct": 100.0, in the example JSON block
  • Line 70: - `progressPct` is rounded to one decimal place (0–100).

All three should be changed to historicalSyncProgressPct. The PR updates the Zod schema, the endpoint implementation, the route description string, and the test assertions — but misses the hand-written example response and the bullet-point description in docs/api-reference.md. A consumer reading the docs will get a field name that does not match the actual API response.

OpenAPI/Swagger (routes.ts) description — updated correctly. The description: string on the route now references historicalSyncProgressPct. The Zod schema (ChainProgressSchema) also updated. The Swagger UI at /docs will show the correct name on next rebuild.

No other docs files affected. docs/deployment.md and docs/architecture.md do not reference this field by name.

Overall: One actionable item before merge — update the three occurrences of progressPct in docs/api-reference.md (lines 56, 63, 70) to historicalSyncProgressPct.

@lgahdl
Copy link
Copy Markdown
Contributor Author

lgahdl commented Jun 2, 2026

Review: Rename progressPct → historicalSyncProgressPct

The rename is complete across all four affected locations:

  • src/api/schemas/sync-progress.ts — Zod field renamed
  • src/api/endpoints/sync-progress.ts — handler assignment and local type annotation renamed
  • src/api/routes.ts — OpenAPI description text updated
  • tests/api/sync-progress.test.ts — local ChainProgress type and both assertions updated

No remaining references to progressPct found in src/ or tests/.

This PR is clean.

…eld name

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread docs/api-reference.md
```

- `progressPct` is rounded to one decimal place (0–100).
- `historicalSyncProgressPct` is rounded to one decimal place (0–100).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

historicalSyncProgressPct is not clear yet for me.

I would prefer something like historicalBlocksFetchedPct

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we add then a historicalBlocksProcessedPct as well?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants