Skip to content

Add Restocking tab and system architecture doc - #234

Open
farhanbhagat99 wants to merge 4 commits into
beck-source:mainfrom
farhanbhagat99:new_features
Open

Add Restocking tab and system architecture doc#234
farhanbhagat99 wants to merge 4 commits into
beck-source:mainfrom
farhanbhagat99:new_features

Conversation

@farhanbhagat99

Copy link
Copy Markdown

Adds a budget-driven restocking workflow: a new Restocking tab lets users set an available budget, see items recommended from demand forecasts (prioritized by urgency and rising demand), and submit an order that appears in the Orders tab under a new "Submitted Orders" section with delivery lead time. Backfills unit_cost onto demand_forecasts.json since its SKUs don't overlap inventory.json's, so recommendations couldn't otherwise be costed.

Also adds docs/architecture.html, a static overview of the system's tech stack, architecture, and data flow for onboarding.

Adds a budget-driven restocking workflow: a new Restocking tab lets
users set an available budget, see items recommended from demand
forecasts (prioritized by urgency and rising demand), and submit an
order that appears in the Orders tab under a new "Submitted Orders"
section with delivery lead time. Backfills unit_cost onto
demand_forecasts.json since its SKUs don't overlap inventory.json's,
so recommendations couldn't otherwise be costed.

Also adds docs/architecture.html, a static overview of the system's
tech stack, architecture, and data flow for onboarding.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@farhanbhagat99

Copy link
Copy Markdown
Author

Reviewed this with a fresh pair of eyes (independent pass, not just re-reading what I wrote). Overall the backend is solid and convention-compliant, but there's one real bug in the frontend interaction that undermines the feature's core UX, plus a few things worth tightening before merge.

🔴 Bug — checked-item selection resets on every budget change

client/src/views/Restocking.vueapplyRecommendations():

const applyRecommendations = (data) => {
  recommendedItems.value = data.recommended_items
  totalCost.value = data.total_cost
  checkedSkus.value = new Set(data.recommended_items.map(item => item.sku)) // always re-checks everything
}

This runs from both loadInitial() and every debounced onBudgetInput() fetch. So: uncheck an item, then nudge the slider by even 1px, and the exclusion is silently wiped — everything gets re-checked. That breaks the "uncheck items to exclude them from the order" interaction the UI advertises.

Fix: only seed "all checked" on the very first load; on subsequent budget-driven refetches, preserve existing unchecked state for SKUs that are still present, e.g.:

const applyRecommendations = (data, isInitial = false) => {
  recommendedItems.value = data.recommended_items
  totalCost.value = data.total_cost
  const newSkus = new Set(data.recommended_items.map(i => i.sku))
  checkedSkus.value = isInitial
    ? newSkus
    : new Set([...checkedSkus.value].filter(s => newSkus.has(s)))
}

🟡 Recommended before merge

  • No server-side validation on restocking order items (server/main.pyRestockingOrderItem): quantity/unit_price are trusted from the client with no bounds. Low blast radius here (in-memory demo, resets on restart), but cheap to fix with Field(gt=0) constraints per this repo's own documented Pydantic convention.
  • Float comparison in the budget cutoff (compute_restocking_recommendations): running_total + candidate['line_total'] <= budget compares raw floats with no epsilon, so hitting exactly budget_max can drop the last item by a sub-cent rounding error. test_full_budget_covers_every_candidate uses a < 0.01 tolerance specifically to paper over this — worth a small epsilon in the actual comparison instead.
  • Submitted orders use real wall-clock dates while the rest of the mock dataset (and QUARTER_MAP) is anchored to 2025, and new orders get warehouse: None, category: None. Any active Time Period/Warehouse/Category filter will make the "Submitted Orders" section disappear after placing an order — probably fine for a demo, but worth a quick gut-check since it's an easy "wait, where did my order go?" moment.

🟢 Nice-to-haves

  • Restocking.vue renders {{ item.name }} directly instead of via translateProductName() like Orders.vue — inconsistent with the app's i18n convention (currently invisible since these item names aren't in the ja translation map).
  • docs/architecture.html doesn't mention the new Restocking view/endpoints — reasonable to bundle a docs update with a feature PR, but it's stale on day one as written.
  • Test gaps in tests/backend/test_restocking.py: no negative/zero budget or quantity/unit_price cases, and nothing directly asserting the "increasing-trend-first" priority ordering.

What's good

  • Backend models/endpoints follow existing FastAPI/Pydantic patterns cleanly.
  • Both locale files updated together, good coverage for the new strings.
  • v-for keys use stable IDs (item.sku, order.id) throughout — no index-as-key regressions.
  • Tests assert real monotonic behavior (test_increasing_budget_never_decreases_recommendations), not just 200-smoke-tests.

Happy to push a follow-up commit for the checkbox-reset fix + the Pydantic constraints if useful — those are the two I'd actually block on.

farhanbhagat99 and others added 3 commits July 29, 2026 11:16
Unchecking a recommended item, then moving the budget slider at all,
silently re-checked every item on the next recommendation refetch --
the checked-item selection was rebuilt from scratch on every budget
change instead of only on first load. Now only the initial load
defaults everything to checked; subsequent refetches preserve the
user's existing exclusions and only default newly-appearing items to
checked. Also adds Pydantic constraints (positive quantity, non-
negative unit price, capped item-list length) to the restocking order
submission endpoint, which previously trusted client-supplied values
with no bounds.

Found during PR review: beck-source#234 (comment)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.claude/settings.local.json holds personal env var overrides
(e.g. experimental feature flags) that shouldn't be shared via git.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Reports page was the one view in the app that hadn't kept pace
with the rest of the codebase's conventions:

- No i18n at all (hardcoded English strings, $ currency symbol
  regardless of locale) -- now fully translated via t(), including
  localized month labels.
- Completely ignored the global filter bar -- neither the frontend
  nor the /api/reports/quarterly and /api/reports/monthly-trends
  backend endpoints accepted warehouse/category/status/month filters.
  Both endpoints now reuse the existing apply_filters/filter_by_month
  helpers, and the view wires into useFilters() like every other view.
- ~14 unconditional console.log calls, several inside per-render
  helper functions -- removed entirely.
- Still on Options API while every other view uses Composition API;
  bypassed the centralized api.js client in favor of raw axios calls
  to a hardcoded URL; used array index as v-for :key (a documented
  anti-pattern in this repo's own CLAUDE.md); recomputed max revenue
  by rescanning the full dataset on every bar render (O(n^2)); had a
  latent bug in hand-rolled number formatting that mis-placed a comma
  on negative values. All fixed to match Orders.vue/Demand.vue
  conventions.

Also translates the "Reports" nav tab, the one nav link that was
still hardcoded in English.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@farhanbhagat99

Copy link
Copy Markdown
Author

Pushed fixes for both blocking items from the review above:

  • 6d83e89 — fixes the checkbox-reset bug (unchecking an item now survives subsequent budget-slider changes; only the very first load defaults everything to checked) and adds Pydantic validation (quantity > 0, unit_price >= 0, item list capped at 100) to POST /api/restocking/orders. Added corresponding tests.
  • 9796008 — unrelated pre-existing Reports page bugs found and fixed in a separate pass (no i18n, ignored the global filter bar, console spam, Options-API/index-key/O(n²) inconsistencies vs. the rest of the app). Bundled into this branch since it was already open; happy to split into its own PR if preferred.

All 62 backend tests pass, production build is clean, and both fixes were verified live in the browser (checkbox exclusion persists across slider moves; Reports page now translates, filters, and is silent in the console).

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