feat(offer): Wallapop 'hacer oferta' flow — operator-tapped offers + negotiable band - #55
Merged
Conversation
…negotiable band Implements OpenSpec change wallapop-make-offer (FR50-FR57): - 💰 Ofertar button (operator-tap only, FR29 extended): tap → offer preflight → re-fetch by internal id → amount recompute with tolerance → TinyFish drives the native offer form with EXACTLY the bounded amount → 'Oferta enviada' / closed 12-variant failure taxonomy. - Negotiable-band alerts: Wallapop listings over ceiling but within ceiling×(1+offer.band_pct) on offer-enabled entries stop being silently filtered (new 'negotiable' phase, 💰 severity token). - Amount = largest whole-euro item price whose buyer total fits the entry target (offer.target_total_eur, default ceiling), bounded by Wallapop's -30% floor (operator-verified app+web, 2026-07-22/23). - Guardrails mirror the buy path: per-listing dedupe (one offer ever), rolling-24h daily budget (default 5, under Wallapop's 10/day cap), independent lockout (offer_state, migration 0004, append-only offers table), keyboard restored on every outcome, kill switch. - Wishlist offer: block + salvager offer enable/disable/status CLI; is_refurbished projected from the search payload and pre-filtered. - Variant registry 45→66 with golden snapshots; PRD amendment FR50-57. Zero behaviour change until an entry opts in via 'salvager offer enable'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djngz3hScLAcRoyKtBCZgb
📝 WalkthroughWalkthroughAdds an opt-in, operator-triggered Wallapop “hacer oferta” flow with computed pricing, negotiable-band alerts, TinyFish execution, Telegram callbacks, audit persistence, lockout controls, CLI commands, daemon wiring, and extensive validation. ChangesWallapop offer capability
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Telegram
participant CallbackDispatcher
participant OfferOrchestrator
participant OfferPreflight
participant WallapopOfferFlow
participant OfferAuditWriter
Operator->>Telegram: Tap Ofertar
Telegram->>CallbackDispatcher: offer callback
CallbackDispatcher->>OfferOrchestrator: execute offer
OfferOrchestrator->>OfferPreflight: check eligibility
OfferOrchestrator->>WallapopOfferFlow: submit reconciled amount
WallapopOfferFlow-->>OfferOrchestrator: success or failure
OfferOrchestrator->>OfferAuditWriter: append attempt
OfferOrchestrator->>Telegram: send outcome and restore keyboard
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI runs 'ruff format --check .' and 'mypy src tests' (wider than the local gate I ran): format the goal-text edit and annotate the new offer test helpers/fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djngz3hScLAcRoyKtBCZgb
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/salvager/orchestration/poll_loop.py (1)
409-475: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winAlready-offered negotiable listings are re-evaluated by the LLM every cycle, forever.
_filter_over_ceiling's negotiable-band condition never checks the offer dedupe (has_offered/has_successful_offer);has_offeredisn't even threaded into this function. Every negotiable-eligible listing therefore pays an LLM-eval cost every cycle regardless of whether it was already successfully offered. The dedupe only happens downstream in_dispatch_alert(line 733-741), which then skips silently viareturn False— but unlike every other skip/drop branch in this module (e.g. Line 489, Line 338), that skip path never callsstore.record_seen, so the listing is never marked seen and repeats this cycle-after-cycle indefinitely (until it ages out of search results or drops under the ceiling).Thread
has_offeredinto_filter_over_ceilingand exclude/record-seen already-offered listings there — this both saves the wasted LLM eval and closes the missingrecord_seengap in one place.🩹 Proposed fix (sketch)
async def _filter_over_ceiling( buyable: list[Listing], entry: WishlistEntry, store: Store, summary: PollCycleSummary, *, assumed_shipping_eur: Decimal, assumed_import_charges_eur: Decimal, marketplace: Marketplace, log: object, offer_band_pct: Decimal | None = None, + has_offered: Callable[[str, str], Awaitable[bool]] | None = None, ) -> tuple[list[Listing], list[Listing]]: ... if ( offer_band_pct is not None and entry.offer.enabled and listing.marketplace == "wallapop" and not listing.is_refurbished and total <= ceiling * (1 + offer_band_pct) and offer_item_price_eur(...) is not None + and not ( + has_offered is not None + and await has_offered(listing.marketplace, listing.listing_id) + ) ): negotiable.append(listing) ...And pass
has_offered=has_offeredat the_filter_over_ceilingcall site (Line 278-288), which already has it available.Also applies to: 733-741
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/salvager/orchestration/poll_loop.py` around lines 409 - 475, Thread the existing has_offered state into _filter_over_ceiling and its call site, then exclude listings already offered from the negotiable bucket while calling store.record_seen for them. Preserve normal within-ceiling handling and ensure the downstream _dispatch_alert dedupe path is no longer responsible for these listings.
🧹 Nitpick comments (3)
src/salvager/orchestration/offer_orchestrator.py (2)
494-497: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
assertused to enforce a domain invariant consumed immediately after.
ceilingfeeds straight into Decimal arithmetic (target = entry.offer.target_total_eur or _entry_ceiling(entry)); if wishlist validation ever regresses and-Ois used, this silently returnsNoneinstead of failing loudly.♻️ Proposed fix
def _entry_ceiling(entry: WishlistEntry) -> Decimal: ceiling = entry.max_price_solo or entry.max_price_in_device - assert ceiling is not None, "wishlist validation guarantees a ceiling" + if ceiling is None: + raise ValueError(f"entry {entry.entry_key} has no price ceiling — wishlist validation should have caught this") return ceiling🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/salvager/orchestration/offer_orchestrator.py` around lines 494 - 497, Replace the assert in _entry_ceiling with explicit runtime validation so a missing ceiling always fails before Decimal arithmetic, including when Python runs with -O. Preserve the existing preference for max_price_solo over max_price_in_device and raise an appropriate exception with clear context when both are absent.
1-1: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winException text is truncated in logs but not in the ctx that reaches the operator's Telegram alert. All three sites log
str(exc)(truncated to 200 chars where shown) but pass the untruncatedstr(exc)into actx["detail"]thatrender_offer_failure/the offer-failure alert eventually renders to the operator — a large or unusual exception message can produce an oversized or noisy Telegram alert.
src/salvager/orchestration/offer_orchestrator.py#L252-260: truncatefail_ctx["detail"](e.g.str(exc)[:200]) to match the log line at Line 254.src/salvager/orchestration/offer_orchestrator.py#L400-414: truncatectx["detail"]in_handle_unexpectedthe same way.src/salvager/adapters/tinyfish_browser/wallapop_offer.py#L264-269: truncate theSDKErrorctx"detail": str(exc)before it's attached toOfferSendFailure.ctx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/salvager/orchestration/offer_orchestrator.py` at line 1, Truncate exception details to 200 characters before storing them in operator-facing contexts. Update the failure context at the offer orchestration site, `_handle_unexpected`, and the `SDKError` context in the Wallapop offer adapter so each `ctx["detail"]` uses the same bounded text as the corresponding logs, while preserving the existing alert flow.Source: Learnings
tests/unit/test_audit_writer_append_only.py (1)
74-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated append-only test scaffolding — extract a shared helper.
The new
OfferAuditWriterblock re-implements the same three checks (noupdate_*/delete_*methods, exact public-surface match, static UPDATE/DELETE scan) already written foraudit_writerabove, with an almost identical regex and introspection helper. A small parametrized helper would avoid re-copying this scaffolding for the next*AuditWriter.♻️ Sketch: shared parametrized helper
def _public_methods(cls: type) -> set[str]: return { name for name, _ in inspect.getmembers(cls, predicate=inspect.isfunction) if not name.startswith("_") } def _assert_no_sql_mutates(module, tables: frozenset[str]) -> None: source = inspect.getsource(module) mutated = re.findall(r"\b(?:UPDATE|DELETE\s+FROM)\s+([a-z_]+)", source, flags=re.IGNORECASE) offenders = sorted(t for t in mutated if t in tables) assert offenders == [], f"append-only tables must never be UPDATE/DELETE'd: {offenders}" `@pytest.mark.parametrize`( ("module", "cls", "tables", "expected_methods"), [ (audit_writer, Phase2AuditWriter, _APPEND_ONLY_TABLES, _EXPECTED_PUBLIC_METHODS), (offer_writer, OfferAuditWriter, _OFFER_APPEND_ONLY_TABLES, _EXPECTED_OFFER_PUBLIC_METHODS), ], ) def test_append_only_contract(module, cls, tables, expected_methods) -> None: methods = _public_methods(cls) assert not any(m.startswith(("update_", "delete_")) for m in methods) assert methods == set(expected_methods) _assert_no_sql_mutates(module, tables)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_audit_writer_append_only.py` around lines 74 - 130, Extract the duplicated append-only checks into shared helpers near the existing audit-writer tests: use a generic _public_methods helper and an _assert_no_sql_mutates helper, then replace the separate Phase2AuditWriter and OfferAuditWriter tests with one parametrized test_append_only_contract covering each module, class, table set, and expected method set. Preserve the existing forbidden-method, exact-public-surface, and SQL-mutation assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@_bmad-output/planning-artifacts/prd.md`:
- Around line 794-805: Resolve the duplicate requirement identifiers in the PRD
by renumbering the pre-existing SIGTERM and distribution requirements
immediately following the Wallapop offer-flow amendment, preserving FR50–FR57
exclusively for the offer requirements. Update all references to the renumbered
requirements across the specification, tasks, and README so traceability remains
consistent.
In `@openspec/changes/wallapop-make-offer/design.md`:
- Around line 31-32: Update the negotiable-band predicate documentation to
require both band membership and a computable floor-valid offer before retaining
a listing. Apply this clarification in
openspec/changes/wallapop-make-offer/design.md lines 31-32,
openspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.md lines
5-7, and README.md lines 191-192; explicitly note that some otherwise in-band
listings remain filtered when no valid offer meets the floor.
In `@openspec/changes/wallapop-make-offer/proposal.md`:
- Line 39: Update the proposal’s configuration module reference from
config/config_yaml.py to src/salvager/config/config_yaml.py, while preserving
the existing references to the example configuration files and bundled template.
In `@src/salvager/cli/app.py`:
- Around line 924-949: Update cmd_offer_status to load the configured
offer.daily_limit from config.yaml and pass it explicitly as daily_limit to
offer_cmd.run_status, rather than relying on run_status’s hardcoded default.
Preserve the existing wishlist path, data directory, output format, and
exit-code handling.
In `@src/salvager/cli/commands/offer_cmd.py`:
- Around line 241-248: Update the offer status command wiring in app.py to pass
the configured offer.daily_limit value into run_status instead of relying on its
default of 5. Preserve run_status’s existing footer and JSON reporting behavior,
using the configured value for both outputs.
In `@src/salvager/domain/listing.py`:
- Around line 67-72: Ensure every Listing projection, especially the detail
construction in WallapopFetcher, preserves the source refurbishment status
instead of defaulting missing values to False. Update the offer eligibility gate
to allow offers only when is_refurbished is explicitly False, using a tri-state
representation if the source value can be unknown.
In `@src/salvager/orchestration/offer_orchestrator.py`:
- Around line 300-325: Handle exceptions from _record_attempt in the
OfferSuccess branch separately from send failures. After execute_offer succeeds,
report audit persistence failure as a critical degradation and return a
success-shaped or dedicated “sent but unaudited” outcome instead of routing
through _fail, resetting/rearming retry state, or presenting the offer as
unsent. Preserve normal audit, counter-reset, and dispatch behavior when
recording succeeds.
---
Outside diff comments:
In `@src/salvager/orchestration/poll_loop.py`:
- Around line 409-475: Thread the existing has_offered state into
_filter_over_ceiling and its call site, then exclude listings already offered
from the negotiable bucket while calling store.record_seen for them. Preserve
normal within-ceiling handling and ensure the downstream _dispatch_alert dedupe
path is no longer responsible for these listings.
---
Nitpick comments:
In `@src/salvager/orchestration/offer_orchestrator.py`:
- Around line 494-497: Replace the assert in _entry_ceiling with explicit
runtime validation so a missing ceiling always fails before Decimal arithmetic,
including when Python runs with -O. Preserve the existing preference for
max_price_solo over max_price_in_device and raise an appropriate exception with
clear context when both are absent.
- Line 1: Truncate exception details to 200 characters before storing them in
operator-facing contexts. Update the failure context at the offer orchestration
site, `_handle_unexpected`, and the `SDKError` context in the Wallapop offer
adapter so each `ctx["detail"]` uses the same bounded text as the corresponding
logs, while preserving the existing alert flow.
In `@tests/unit/test_audit_writer_append_only.py`:
- Around line 74-130: Extract the duplicated append-only checks into shared
helpers near the existing audit-writer tests: use a generic _public_methods
helper and an _assert_no_sql_mutates helper, then replace the separate
Phase2AuditWriter and OfferAuditWriter tests with one parametrized
test_append_only_contract covering each module, class, table set, and expected
method set. Preserve the existing forbidden-method, exact-public-surface, and
SQL-mutation assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95b35c96-35db-4036-85a3-8f429ccc682b
⛔ Files ignored due to path filters (4)
openspec/changes/wallapop-make-offer/captures/app-listing-hacer-oferta-button.jpgis excluded by!**/*.jpgopenspec/changes/wallapop-make-offer/captures/app-offer-form-10-restantes.jpgis excluded by!**/*.jpgopenspec/changes/wallapop-make-offer/captures/app-offer-form-floor-30pct.jpgis excluded by!**/*.jpguv.lockis excluded by!**/*.lock
📒 Files selected for processing (60)
README.md_bmad-output/planning-artifacts/prd.mdconfig.example.yamldocs/release-audits/v1.0/SUMMARY.mdopenspec/changes/wallapop-make-offer/.openspec.yamlopenspec/changes/wallapop-make-offer/captures/NOTES.mdopenspec/changes/wallapop-make-offer/design.mdopenspec/changes/wallapop-make-offer/proposal.mdopenspec/changes/wallapop-make-offer/specs/listing-alert-state-updates/spec.mdopenspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.mdopenspec/changes/wallapop-make-offer/specs/wallapop-offer-flow/spec.mdopenspec/changes/wallapop-make-offer/tasks.mdopenspec/config.yamlsrc/salvager/adapters/sqlite_store/offer_writer.pysrc/salvager/adapters/telegram_bot/surface.pysrc/salvager/adapters/tinyfish_browser/__init__.pysrc/salvager/adapters/tinyfish_browser/wallapop_offer.pysrc/salvager/adapters/wallapop_api/fetcher.pysrc/salvager/adapters/wallapop_api/schema.pysrc/salvager/cli/app.pysrc/salvager/cli/commands/dev_cmd.pysrc/salvager/cli/commands/offer_cmd.pysrc/salvager/cli/dev_alert_fixtures.pysrc/salvager/config/config_yaml.pysrc/salvager/config/wishlist_yaml.pysrc/salvager/domain/alert.pysrc/salvager/domain/audit.pysrc/salvager/domain/errors.pysrc/salvager/domain/listing.pysrc/salvager/domain/offer_audit.pysrc/salvager/domain/pricing.pysrc/salvager/domain/wishlist.pysrc/salvager/interfaces/offer_session.pysrc/salvager/migrations/0004_offer_schema.sqlsrc/salvager/orchestration/alert_updater.pysrc/salvager/orchestration/callback_handler.pysrc/salvager/orchestration/composer.pysrc/salvager/orchestration/offer_orchestrator.pysrc/salvager/orchestration/offer_preflight.pysrc/salvager/orchestration/poll_loop.pysrc/salvager/templates/config.example.yamlsrc/salvager/templates/wishlist.example.yamltests/e2e/test_poll_loop_offer_band.pytests/unit/__snapshots__/test_offer_renderer_snapshots.ambrtests/unit/__snapshots__/test_operational_alert_renderer.ambrtests/unit/test_alert_renderer.pytests/unit/test_audit_writer_append_only.pytests/unit/test_callback_handler.pytests/unit/test_cli_offer.pytests/unit/test_dev_emit_alert.pytests/unit/test_offer_orchestrator.pytests/unit/test_offer_pricing.pytests/unit/test_offer_renderer_snapshots.pytests/unit/test_offer_writer.pytests/unit/test_operational_alert_renderer.pytests/unit/test_phase2_schema.pytests/unit/test_sqlite_store.pytests/unit/test_tinyfish_offer_flow.pytests/unit/test_wallapop_api_fetcher.pywishlist.example.yaml
Comment on lines
+794
to
805
|
|
||
| **Wallapop offer flow (wallapop-make-offer amendment, 2026-07-22):** | ||
|
|
||
| - **FR50.** The operator can send a price offer ("hacer oferta") on a Wallapop listing by tapping a `💰 Ofertar` inline button; no offer is ever sent without an operator tap (the FR29 no-autonomous-action rule extends to offers). | ||
| - **FR51.** The offer amount is computed, not chosen: the largest whole-euro item price whose delivered buyer total fits the entry's offer target (`offer.target_total_eur`, defaulting to the entry ceiling), bounded by Wallapop's platform floor of 70 % of the asking price, shown on the alert before the tap and recomputed from the reconciled listing at tap time. | ||
| - **FR52.** Wallapop listings on offer-enabled entries whose buyer total exceeds the ceiling but sits within `ceiling × (1 + offer.band_pct)` produce a distinct negotiable alert (`💰` severity token, offer row, `Ofertar · Saltar · Ver` keyboard, never Comprar) instead of being silently filtered; listings beyond the band, on offer-disabled entries, or on eBay filter exactly as before. | ||
| - **FR53.** Offer sending enforces a self-imposed daily budget (`offer.daily_limit`, default 5 per rolling 24 h, deliberately under Wallapop's 10-per-calendar-day account cap) and recognises the platform's own exhausted-counter state; neither limit-hit increments the failure lockout. | ||
| - **FR54.** At most one successful offer is ever sent per listing (per-listing dedupe); a listing with a sent offer keeps a terminal `💰 Oferta enviada` badge across keyboard reconstructions. | ||
| - **FR55.** Offer outcomes use the closed `OfferFailureReason` set, each variant rendered with a Spanish cause label, detail rows, next steps, and the reassurance line "No se ha enviado ninguna oferta." (with a documented ambiguity variant for missing confirmation evidence); v1 ends at "offer sent" — seller responses are handled by the operator in the Wallapop app. | ||
| - **FR56.** Consecutive offer execution failures reaching `offer.lockout_threshold` disable the offer path globally until `salvager offer enable <entry>` clears the lockout; the offer lockout and `offer.kill_switch_global` are fully independent from the Phase 2 circuit breaker and kill switch. | ||
| - **FR57.** Offers are opt-in per wishlist entry (`offer.enabled`, default false, toggled via `salvager offer enable/disable`); with no entry opted in, alert filtering, rendering, and callbacks are byte-identical to the pre-offer behaviour, and every executed offer attempt is recorded in the append-only `offers` audit table. | ||
| - **FR50.** The agent handles SIGTERM gracefully — drains in-flight LLM evaluations, flushes the audit log, completes pending Telegram alerts, exits within 30 seconds. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Duplicate FR ids: FR50-FR54 now used twice with unrelated content.
The new offer-flow amendment claims FR50-FR57, but FR50 (SIGTERM handling, line 805) and FR51-FR54 (distribution artifacts, lines 809-812) already exist unchanged immediately below with the same ids. This breaks the FR-traceability contract this section states — every doc referencing "FR50-FR57" for the offer flow (the spec files, tasks.md, README) now also collides with the SIGTERM/distribution requirements.
🔧 Proposed fix: renumber the pre-existing FRs that follow the insertion point
-- **FR50.** The agent handles SIGTERM gracefully — drains in-flight LLM evaluations, flushes the audit log, completes pending Telegram alerts, exits within 30 seconds.
+- **FR58.** The agent handles SIGTERM gracefully — drains in-flight LLM evaluations, flushes the audit log, completes pending Telegram alerts, exits within 30 seconds.
### Project Distribution & Artifacts
-- **FR51.** The repository ships a single `docker-compose.yml` install path with example wishlist entries for common HDD and RAM models, an `.env.example`, and a `config.example.yaml`; user-specific files (`wishlist.yaml`, `config.yaml`, `.env`) are gitignored.
-- **FR52.** The repository includes a `CONTRIBUTING.md` with an explicit "no arbitrage PRs" rule and three named invitation categories (wishlist examples, prompt improvements, Wallapop selector patches), pointing to a separate-repo path for arbitrage forks.
-- **FR53.** The repository includes a `ROADMAP.md` naming future-multi-marketplace expansion, future-arbitrage-as-separate-repo, and "C&D-induced sunset" as a documented possible end state.
-- **FR54.** The README positions salvager as a personal monitoring tool (not a "Wallapop scraper"), includes a legal disclaimer covering Spanish ToS posture and the secondary-account recommendation, and contains no Wallapop trademarks, logos, or proprietary terms in titles, package names, or domain references.
+- **FR59.** The repository ships a single `docker-compose.yml` install path with example wishlist entries for common HDD and RAM models, an `.env.example`, and a `config.example.yaml`; user-specific files (`wishlist.yaml`, `config.yaml`, `.env`) are gitignored.
+- **FR60.** The repository includes a `CONTRIBUTING.md` with an explicit "no arbitrage PRs" rule and three named invitation categories (wishlist examples, prompt improvements, Wallapop selector patches), pointing to a separate-repo path for arbitrage forks.
+- **FR61.** The repository includes a `ROADMAP.md` naming future-multi-marketplace expansion, future-arbitrage-as-separate-repo, and "C&D-induced sunset" as a documented possible end state.
+- **FR62.** The README positions salvager as a personal monitoring tool (not a "Wallapop scraper"), includes a legal disclaimer covering Spanish ToS posture and the secondary-account recommendation, and contains no Wallapop trademarks, logos, or proprietary terms in titles, package names, or domain references.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@_bmad-output/planning-artifacts/prd.md` around lines 794 - 805, Resolve the
duplicate requirement identifiers in the PRD by renumbering the pre-existing
SIGTERM and distribution requirements immediately following the Wallapop
offer-flow amendment, preserving FR50–FR57 exclusively for the offer
requirements. Update all references to the renumbered requirements across the
specification, tasks, and README so traceability remains consistent.
Comment on lines
+31
to
+32
| **D3 — Negotiable band = a carve-out at the existing over-ceiling filter, upstream of evaluation.** | ||
| `poll_loop._filter_over_ceiling` (pre-eval) gains one branch: a Wallapop listing on an offer-enabled entry whose buyer total is over the ceiling but ≤ `ceiling × (1 + offer.band_pct)` (config, default 0.20) is tagged *negotiable* and kept; everything beyond the band is dropped exactly as today. Tagged listings flow through the unchanged LLM evaluation and confidence gate — junk in the band must not alert just because it is cheap-ish. At render time the tag selects the negotiable renderer: distinct severity token, the standard buyer-total breakdown, plus an offer line (`💰 Oferta: 74 € (total ≤ 80,00 €)` shape) and the Ofertar row — never Comprar (over ceiling by definition; the Phase 2 preflight would reject it anyway). *Cost note:* the band widens the LLM-eval funnel; bounded by per-entry opt-in and the band width. Seen-listing dedupe applies unchanged. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the negotiable-band predicate explicit everywhere.
Runtime filtering requires both band membership and a computable floor-valid offer, while these documents describe band membership alone.
openspec/changes/wallapop-make-offer/design.md#L31-L32: add the valid-offer condition.openspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.md#L5-L7: update the carve-out requirement.README.md#L191-L192: explain why some in-band listings remain filtered.
📍 Affects 3 files
openspec/changes/wallapop-make-offer/design.md#L31-L32(this comment)openspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.md#L5-L7README.md#L191-L192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openspec/changes/wallapop-make-offer/design.md` around lines 31 - 32, Update
the negotiable-band predicate documentation to require both band membership and
a computable floor-valid offer before retaining a listing. Apply this
clarification in openspec/changes/wallapop-make-offer/design.md lines 31-32,
openspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.md lines
5-7, and README.md lines 191-192; explicitly note that some otherwise in-band
listings remain filtered when no valid offer meets the floor.
…renumber, status denominator - CRITICAL: a failed offers-table write AFTER a verified send no longer masquerades as a send failure (which rendered the false reassurance, counted a lockout failure, and left the dedupe unrecorded). The outcome stays success; the missing audit row escalates via an offer_orchestrator_error operational alert. - Renumber the offer PRD amendment FR50-FR57 → FR58-FR65 (FR50-FR54 were already taken by the ops/repo sections) and update every reference in code, tests, and specs. - 'offer status' now threads config.offer.daily_limit into the budget denominator (--config-path, best-effort load). - fetch_listing carries the search-derived is_refurbished forward (the detail payload doesn't expose the flag) so offer eligibility never widens on a re-fetch. - Spec deltas state the negotiable-band predicate explicitly (band AND computable floor-valid offer); proposal path typo fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Djngz3hScLAcRoyKtBCZgb
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/salvager/adapters/wallapop_api/fetcher.py`:
- Around line 305-310: Preserve the prior reservation state in the detail
re-fetch mapping alongside is_refurbished: pass listing.is_reserved into the
constructed Listing because WallapopApiItemDetail does not provide that field,
while allowing a future API value to override it if available. Add a regression
test covering a reserved listing through detail re-fetch and verifying the offer
flow remains ineligible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8471ef25-45ba-4431-af46-b7c0f345674d
📒 Files selected for processing (15)
_bmad-output/planning-artifacts/prd.mdopenspec/changes/wallapop-make-offer/proposal.mdopenspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.mdopenspec/changes/wallapop-make-offer/specs/wallapop-offer-flow/spec.mdsrc/salvager/adapters/tinyfish_browser/wallapop_offer.pysrc/salvager/adapters/wallapop_api/fetcher.pysrc/salvager/cli/app.pysrc/salvager/cli/commands/offer_cmd.pysrc/salvager/domain/alert.pysrc/salvager/interfaces/offer_session.pysrc/salvager/orchestration/offer_orchestrator.pysrc/salvager/orchestration/offer_preflight.pytests/unit/test_alert_renderer.pytests/unit/test_offer_orchestrator.pytests/unit/test_offer_renderer_snapshots.py
🚧 Files skipped from review as they are similar to previous changes (13)
- tests/unit/test_offer_renderer_snapshots.py
- openspec/changes/wallapop-make-offer/specs/shipping-aware-pricing/spec.md
- tests/unit/test_alert_renderer.py
- src/salvager/cli/app.py
- src/salvager/interfaces/offer_session.py
- _bmad-output/planning-artifacts/prd.md
- src/salvager/adapters/tinyfish_browser/wallapop_offer.py
- src/salvager/cli/commands/offer_cmd.py
- src/salvager/orchestration/offer_preflight.py
- openspec/changes/wallapop-make-offer/specs/wallapop-offer-flow/spec.md
- tests/unit/test_offer_orchestrator.py
- src/salvager/orchestration/offer_orchestrator.py
- src/salvager/domain/alert.py
Comment on lines
+305
to
+310
| # The detail payload does NOT expose the refurbished flag | ||
| # (live-probed 2026-07-22) — carry the search-derived value | ||
| # forward instead of silently resetting it to False, so the | ||
| # offer path's eligibility never widens on a re-fetch. The | ||
| # agent goal's missing-button check is the runtime backstop. | ||
| is_refurbished=listing.is_refurbished, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A25 -B5 'is_reserved|class Listing' src/salvager/domain/listing.py src/salvager/adapters/wallapop_api/schema.py tests/unit/test_wallapop_api_fetcher.py tests/unit/test_offer_orchestrator.pyRepository: ifuensan/salvager
Length of output: 15778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the Wallapop fetcher around the detail reconciliation and the offer gate.
sed -n '250,390p' src/salvager/adapters/wallapop_api/fetcher.py
printf '\n----\n'
sed -n '1,220p' src/salvager/domain/listing.py
printf '\n----\n'
sed -n '1,220p' src/salvager/domain/offer_orchestrator.py
printf '\n----\n'
rg -n -A8 -B4 'reserved|is_reserved|detail' src/salvager/adapters/wallapop_apiRepository: ifuensan/salvager
Length of output: 10563
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the detail payload schema and any tests covering detail re-fetch behavior.
rg -n -A12 -B6 'class WallapopApiItemDetail|reserved|is_reserved|is_refurbished|detail' src/salvager/adapters/wallapop_api/schema.py tests/unit/test_wallapop_api_fetcher.pyRepository: ifuensan/salvager
Length of output: 20256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the detail schema and the code that builds a Listing from detail data.
sed -n '1,220p' src/salvager/adapters/wallapop_api/schema.py
printf '\n----\n'
sed -n '250,360p' src/salvager/adapters/wallapop_api/fetcher.py
printf '\n----\n'
rg -n -A6 -B4 'WallapopApiItemDetail|reserved|is_reserved|detail' src/salvager/adapters/wallapop_api/schema.py src/salvager/adapters/wallapop_api/fetcher.pyRepository: ifuensan/salvager
Length of output: 23522
Preserve is_reserved during detail re-fetch. WallapopApiItemDetail has no reserved field, and Listing.is_reserved defaults to False; dropping it here can turn a reserved listing into an eligible one and let the offer flow proceed. Carry the prior value through (or map it if the API adds it later) and add a regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/salvager/adapters/wallapop_api/fetcher.py` around lines 305 - 310,
Preserve the prior reservation state in the detail re-fetch mapping alongside
is_refurbished: pass listing.is_reserved into the constructed Listing because
WallapopApiItemDetail does not provide that field, while allowing a future API
value to override it if available. Add a regression test covering a reserved
listing through detail re-fetch and verifying the offer flow remains ineligible.
This was referenced Jul 23, 2026
ifuensan
added a commit
that referenced
this pull request
Jul 23, 2026
Version bump + CHANGELOG entry for #55/#56; README status blurb and recommended pinned tag, ROADMAP burn-in tag → 0.5.0. Claude-Session: https://claude.ai/code/session_01Djngz3hScLAcRoyKtBCZgb Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the OpenSpec change
wallapop-make-offerend-to-end (29/29 tasks; artifacts inopenspec/changes/wallapop-make-offer/, PRD amendment FR50–FR57).What this adds
💰 Oferta enviada(or one of 12 closedOfferFailureReasonvariants, each fully rendered with the "No se ha enviado ninguna oferta." reassurance).ceiling × (1 + offer.band_pct)(default +20 %) on offer-enabled entries stop being silently filtered — newnegotiablephase,💰severity token,Ofertar · Saltar · Verkeyboard, never Comprar. LLM evaluation + confidence gate unchanged.offer.target_total_eur, default = ceiling), bounded by Wallapop's platform floor of 70 % of asking. Shown on the alert before the tap; drift at tap time aborts fail-closed.offer.daily_limit, default 5 — under Wallapop's 10/day account cap, whose on-form counter the agent captures into the audit row), independent lockout with its ownoffer_staterow + kill switch (offer failures never block buys), append-onlyoffersaudit table (migration0004), and the keyboard restored on EVERY outcome (the v0.4.3 lesson, inherited).salvager offer enable <ref> [-t target]/disable [--all]/status(enable clears the lockout; status shows budget + lockout).openspec/changes/wallapop-make-offer/captures/): listing-page button,/app/chat/offer?itemId=<internal id>form, comma-decimal amount field, 10/day counter, −30 % floor. Live API probe:is_refurbishedexposed (now projected + pre-filtered — refurbished listings never render a dead button); no PRO flag (covered byoffer_unavailable).Compatibility
With no
offer:block on any wishlist entry (the default), filtering, rendering, callbacks, and the daemon lifecycle are byte-identical to v0.4.4 — verified by the untouched pre-existing snapshot suites. Migration 0004 is additive; rollback = previous image (new tables ignored).Test plan
/appsandbox failures excepted),openspec validate --strictOK.OfferAuditWriter, negotiable-band e2e (7 cases incl. byte-identical off-path), TinyFish offer-flow adapter tests (18), orchestrator tests (12: every abort path, lockout at threshold, budget, keyboard restore), CLI tests (12), 34 golden snapshots for all 17 offer rendering surfaces (registry 45→66, pins updated with derivation).docs/release-audits/v1.0/SUMMARY.md("Pending delta").🤖 Generated with Claude Code
https://claude.ai/code/session_01Djngz3hScLAcRoyKtBCZgb
Summary by CodeRabbit