Skip to content

fix: price-check limit sizes only the sample; serialise reads on the shared tab - #5

Merged
mabry1985 merged 1 commit into
mainfrom
fix/price-check-fixes
Oct 6, 2026
Merged

mabry1985 merged 1 commit into
mainfrom
fix/price-check-fixes

Conversation

@mabry1985

Copy link
Copy Markdown
Member

Summary

E1 — limit shrank the statistics. ebay_price_check and amazon_price_check did listings = listings[:cap] before summarize() / results_found / _record(), so limit=1 reported results_found: 1 and a one-row median, contradicting "statistics cover every result found". Now the stats, results_found and the price-history record cover the full set (still bounded by max_results, as ebay_price_and_profit already was); limit only sizes the returned sample (default 8). Documented in both docstrings.

E2 — concurrent calls read each other's pages. All tools share one Browser / labelled tab, and the host runs sync tools on worker threads, so parallel ebay_price_check calls interleaved navigate → wait → read and reported another query's comps.

  • _read_page now holds a per-session threading.Lock for the whole navigate/settle/retry sequence.
  • A read that is still hijacked after the single retry now raises EbayError (the docstring promised "a second hijack is reported"; the code returned the foreign data). ebay_page_probe and ebay_session_status pass strict=False and keep reporting it their own way.
  • _hijacked compares the search's filters too — LH_Sold/LH_Complete, LH_ItemCondition, _udlo/_udhi (Amazon: low-price/high-price) — not just the keywords. Sort (_sop / s) is judged only when both pages state one, so an implicit default sort is not a false hijack.

One existing fixture used a landed URL without the request's filters (?_nkw=x answering a sold search); it now lands on the real search_url(..., sold=True).

Tests

ruff check . && ruff format --check . clean; pytest -q → 189 passed (172 before + 17 new). The 12 new behavioural tests fail against the old tools.py:

  • limit trims only the sample (eBay + Amazon, incl. history row count)
  • second hijack → error, retried exactly once; probe still reports instead of failing
  • filter-mismatch hijack matrix (sold vs active, condition, price band, sort, Amazon)
  • 6 threads on a shared stub tab: max one navigate→read in flight, each result carries its own query

No version bump / release.

🤖 Generated with Claude Code

…shared tab

- ebay_price_check / amazon_price_check: `limit` truncated the listings before
  summarize() and results_found, so limit=1 reported one result and a one-row
  median. Statistics, results_found and the history record now cover the full
  set; `limit` only sizes the returned sample (default 8).
- Concurrent tool calls (sync tools run on worker threads) interleaved
  navigate -> wait -> read on the one labelled tab and read each other's pages.
  The read sequence now holds a per-session threading.Lock.
- A read still hijacked after the one retry is now an error instead of being
  priced; the probe and session-status keep reporting it themselves.
- The same-search check compares filters too (sold/complete, condition, price
  band; sort when both pages state one), not just the keywords.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

QA panel review — PASS

code-review-structural · head dddfb1560c2a · formal

No findings were raised, so there was nothing to verify. The panel (all five finders plus the structural engine) reviewed the diff and found no defects. No disagreements, no coverage gaps, no prior requests to disposition.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Promoting the PASS verdict for head dddfb1560c2a: all checks terminal-green, zero unresolved review threads. (approve-on-green)

@mabry1985
mabry1985 merged commit 1da7401 into main Oct 6, 2026
3 checks passed
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.

1 participant