diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..8c76c32 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,86 @@ +# Changelog + +All notable changes to this project are documented in this file. + +The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +This file starts at 3.6.0 — earlier releases were not retroactively documented. + +## [3.6.0] - 2026-08-03 + +### Fixed + +- **Sidebar navigation was never mounted.** The `Sidebar` component and its + `getLeftRail`/training-rail nav model (`nav/consoleNav.ts`) were fully + built and unit-tested but never rendered into `AdConsole.tsx` — Missions, + Reports, Bulk ops, Trainer, and Integrity were unreachable from the + desktop UI. `MobileNav` had the same gap from a different angle (its + section resolution only ever returned `campaigns`/`portfolio`). Both are + now wired through the shared `sidebarSectionForView`/`isSidebarItemActive`/ + `resolveSidebarClick` helpers. +- **Every Astryx `Card`'s `padding` prop silently rendered as `0px` + sitewide.** Root cause: Astryx ships component styles inside + `@layer astryx-base`/`@layer astryx-theme`, and per the CSS + cascade-layers spec an unlayered declaration always beats a layered one + regardless of specificity. This app's global reset + (`*, *::before, *::after { ...; padding: 0; }`) was unlayered, so it + unconditionally zeroed every Astryx padding prop — this is why content + (buttons, headings, form fields) so often sat flush against card edges. + Scoped the reset down to just `ul`/`ol` (the only elements relying on it). +- Campaign creation wizard's Review & Launch step showed a "Lookback: 30 + days" row on every campaign, including plain Sponsored Products, because + `audienceLookback` defaults to `'30'` regardless of type. Gated it on the + campaign's targeting mode actually being an audience mode. Also added + missing review rows for ASIN/category/audience targets and SB/SD creative + fields (headline, brand, destination) that were entered earlier in the + wizard but never shown before launch. +- `.split`'s `2fr 1fr` grid (dashboard "Operator alerts"/"Training + coverage" cards) didn't shrink to fit once the sidebar took up real + width, clipping content past the viewport edge. Fixed with + `minmax(0, ...)` tracks. +- `adjustTargetBid` (the "-10%"/"+10%" bid buttons) threw an uncaught + error when decrementing an already-cheap bid below the platform + minimum, instead of flooring it — a regression from the bid + fail-fast change below. +- `setTargetBid`/`setAdGroupDefaultBid` no longer silently substitute a + bid below the real $0.02 minimum; they fail fast via a new + `assertValidBid`/`MIN_BID` (`src/lib/validation.ts`), matching this + codebase's existing fail-fast convention. Creation/normalization paths + (`addTarget`, `normalizeCampaign`) still clamp, since those fill in + defaults for incomplete data rather than acting on explicit user intent. +- Campaign Manager's empty state no longer says "No campaigns yet" when a + search/filter simply matched nothing — it now shows a distinct "no + matches" state with a "Clear filters" action. +- Fixed a campaign-ID collision risk in `launchCampaign` (two campaigns of + the same type launched within the same millisecond could get the same + ID) by switching to the shared `generateId` helper, and deduplicated the + same ad-hoc ID-generation pattern across the `profiles`/`reports`/ + `trainer`/`integrity` feature engines. +- Fixed literal mojibake (`ΓÇö`, `ΓåÆ`) in the landing page copy and a + missing `.object-cover` rule that left two landing-page images without + `object-fit` applied. +- Replaced dead Tailwind sizing classes (`w-6 h-6`, `w-4 h-4`) on landing + page icons with explicit SVG dimensions — this repo has no Tailwind + compiler wired up, so the classes were doing nothing. + +### Added + +- Contract tests pinning the sidebar fix (renders, drives navigation to + the previously-unreachable views) and the wizard review-step fix. +- Regression tests for `isVideoFormat`, the shared `generateId` migration + (same-millisecond uniqueness across all four feature engines), and the + simulation's search-term dedup across repeated `simulateDays` calls. + +### Changed + +- Deduplicated hand-rolled metrics/formatter logic in `PortfolioOverview`, + `Dashboard`, and `CampaignManager` onto the shared `totalMetrics`/ + `formatMoney`/`formatWhole`/`formatPercent` engine functions. +- Un-Card-wrapped dense tables in `PortfolioOverview`, `Dashboard`, + `OverviewTab`, `BulkOpsPage`, and `ReportsPage` per this repo's own + convention (dense data renders edge-to-edge, `Card` is for dashboard + widgets/settings groups only). +- Fixed an O(n·m) duplicate-detection loop in the search-term simulator + (now O(1) via a `Set`) and memoized a few expensive per-render + aggregations (`Dashboard`, `ManagerSearchTermsTab`). +- Removed 13 dead `useState` hooks and unused imports from + `CreateCampaignWizard`. diff --git a/CLAUDE.md b/CLAUDE.md index c6520c5..04ae677 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,7 +52,7 @@ Core engine (src/engine/ad-console/core/) — zero framework dependencies, pure **`core/engine/`, `core/types.ts`, and `core/simulation.ts` have zero React/Next/Zustand dependencies.** They are pure TypeScript: given state in, return new state out, no mutation, no side effects. This is the most important invariant in the codebase — it's what makes those modules portable and unit-testable in isolation. Never import React, Next.js, or store code into them. Note that `core/slices/` (below) is the one exception within `core/` — it depends on Zustand's `StateCreator` type by design, since its job is to wrap the pure engine in store slices. - `core/types.ts` — every domain interface (Campaign, AdGroup, Target, Negative, BudgetRule, Portfolio, Metrics, etc.) -- `core/engine/` — one module per domain concern: `campaign.ts`, `target.ts`, `adgroup.ts`, `negative.ts`, `budget.ts`, `portfolio.ts`, `draft.ts`, `id.ts`, `metrics.ts`, `responsive.ts`, `search-term-generator.ts`. All re-exported through `core/engine/index.ts`. +- `core/engine/` — one module per domain concern: `campaign.ts`, `target.ts`, `adgroup.ts`, `negative.ts`, `budget.ts`, `portfolio.ts`, `draft.ts`, `id.ts`, `metrics.ts`, `responsive.ts`, `search-term-generator.ts`. All re-exported through `core/engine/index.ts`. `campaign.ts` also exports `isVideoFormat(type, adFormat)`, the single source of truth for which `adFormat` string means "video" for a given campaign type (SB uses `'Video'`, SD uses `'Video creative'`) — used by both the engine and `OverviewTab`. - `core/simulation.ts` — the 7-day performance simulator; metrics cascade target → ad group → campaign → dashboard. - `core/slices/` — Zustand-dependent `StateCreator` slices (core, target, adgroup, negative, budget, portfolio, draft) that wrap the pure engine functions with state. - `features//` — self-contained modules (`drills`, `profiles`, `trainer`, `bulk`, `reports`, `missions`, `integrity`), each with its own `types.ts`, `engine.ts`, `store.ts`. Adding a feature means adding a new directory here — existing files shouldn't need edits (open/closed). @@ -86,13 +86,15 @@ NextAuth v5 (beta), Credentials provider, JWT sessions, bcrypt password hashing. ### UI conventions (Astryx design system) Components come from `@astryxdesign/core` (153 components, theme via `@astryxdesign/theme-neutral`). This is actively used across the component tree (~40 files) — don't hand-roll layout `
`s or raw CSS when an Astryx component/prop/token covers it. Key rules (full detail lives in `AGENTS.md`'s Astryx block): -- No raw `
` for layout — components handle layout/spacing (`AppShell` for full pages, `SideNav` for sidebar nav). +- No raw `
` for layout — components handle layout/spacing (`AppShell` for full pages, `SideNav` for sidebar nav). In practice the component layer predates full Astryx adoption and still uses hand-rolled `.app-layout`/`.app-sidebar`/`.app-main` divs throughout (see "known SOLID violations" below) — match the existing pattern in a file rather than mixing conventions mid-component. - Dense data → `Table`/`List`/`Item` rows edge-to-edge, never Card-wrapped. `Card` is for dashboard widgets/galleries/settings groups only. - Styling values must be tokens (`var(--color-*|--spacing-*|--radius-*)`) — no raw hex/px, no Tailwind utility classes (this repo has no Tailwind compiler wired up despite Tailwind appearing in some older docs). - Discover components/props via the CLI: `npm run astryx -- component `, `npm run astryx -- search ""`, `npm run astryx -- build ""`. +- **Never add `padding` (or any box-model property Astryx components expose as a prop) to a bare-selector reset in `globals.css`** (e.g. `*, *::before, *::after { ... }`). Astryx ships its component styles inside `@layer astryx-base`/`@layer astryx-theme`; per the CSS cascade-layers spec, *any* unlayered declaration beats a layered one regardless of specificity. An unlayered `* { padding: 0 }` silently zeroed every Astryx `padding` prop sitewide until it was found and fixed (3.6.0) — the global reset only zeroes `margin`, plus `padding` on the couple of native elements (`ul`, `ol`) that actually need it. If a future reset-like rule needs to beat Astryx's own styling, put it in the unlayered `src/app/astryx-theme.css` bridge scoped to the specific class/selector, not a wildcard. +- Empty states use the shared `EmptyState` component (`src/components/AdConsole/details/EmptyState.tsx`) — icon + title + optional message, not a bare `Card` with a muted paragraph. ### Validation -Engine functions fail fast: invalid input throws `ValidationError` (`src/lib/validation.ts`) rather than silently clamping or producing `NaN`. Follow this pattern for new engine functions — don't add silent fallbacks. +Engine functions fail fast: invalid input throws `ValidationError` (`src/lib/validation.ts`) rather than silently clamping or producing `NaN`. Follow this pattern for new engine functions — don't add silent fallbacks. `MIN_BID` and `assertValidBid` (also in `src/lib/validation.ts`) enforce the $0.02 platform bid floor for "set an explicit bid" actions (`setTargetBid`, `setAdGroupDefaultBid`); creation/normalization paths (`addTarget`, `normalizeCampaign`) still clamp instead of throwing, since those fill in defaults for incomplete data rather than acting on explicit user intent. Relative adjustments (`adjustTargetBid`, the "±10%" buttons) floor at `MIN_BID` rather than fail fast, since the caller doesn't fully control the resulting value. ## Testing conventions @@ -117,6 +119,7 @@ These come from `AGENTS.md`, `LOOP.md`, `loop-constraints.md`, and `gate.yaml` - `legacy/` holds the pre-Next.js prototype (a single-file `amazon_ppc_simulator.html` with inline JS) and its old QA/docs — historical reference only, not part of the current build. - `codegraphs/Amazon-ad-console.md` describes that old single-file prototype and is stale relative to the current Next.js/engine architecture described above; don't rely on it. - `docs/` has deeper reference material: `ARCHITECTURE.md`, `API.md` (full engine function signatures), `SCHEMA.md`, `FEATURES.md`, `INTEGRATION.md` (porting guide), `AUTH.md`, `AUDIT-FOLLOWUPS.md`. +- `CHANGELOG.md` (repo root) tracks notable changes per release starting at 3.6.0; bump `version` in `package.json` (and the unused-but-should-stay-in-sync `coreState.version` in `core/slices/core.ts`) together with a new entry when cutting a release. - `skills/`, `patterns/`, `gate.yaml`, `STATE.md`, `loop-*.md` support an autonomous triage/fix loop tool used against this repo — not part of the app runtime. ## Porting the engine diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 70d3d76..e9fd5b7 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -405,13 +405,13 @@ When clicking a tab-mapped item: | Range | Label | Behavior | |-------|-------|----------| -| < 768px | Mobile | Sidebar hidden, hamburger toggle + slide-out drawer | -| 768–1100px | Tablet | Sidebar collapses to 200px, hamburger still available | -| > 1100px | Desktop | Full Amazon Console layout | +| < 768px | Mobile | Desktop sidebar hidden; hamburger toggle + slide-out drawer | +| 768–1100px | Tablet | Same as mobile — desktop sidebar hidden, hamburger drawer takes over (both are `isMobileOrTablet` in `useBreakpoint`) | +| > 1100px | Desktop | Full Amazon Console layout with the persistent left sidebar | ### Mobile Drawer - Hamburger button in the global nav toggles a slide-out drawer -- Drawer contains all sidebar groups: Campaign Manager, Portfolios, Measurement +- Drawer contains all sidebar groups for the active section: Campaign Manager, Portfolios, Measurement, or Training (Drills/Missions/Reports/Bulk ops/Trainer/Integrity) - Backdrop overlay with click-to-close - Escape key closes the drawer - Animation state machine: closed → open ↔ closing → closed diff --git a/package-lock.json b/package-lock.json index 052a641..b9a0e56 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "amazon-ad-console", - "version": "3.5.0", + "version": "3.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "amazon-ad-console", - "version": "3.5.0", + "version": "3.6.0", "hasInstallScript": true, "dependencies": { "@astryxdesign/core": "^0.1.8", diff --git a/package.json b/package.json index 671f052..ae5c609 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "amazon-ad-console", - "version": "3.5.0", + "version": "3.6.0", "private": true, "description": "Amazon PPC Training Simulator — Amazon Ads Console replica for VA training", "scripts": { diff --git a/src/app/globals.css b/src/app/globals.css index 256a5c1..4a6a3cc 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -60,6 +60,8 @@ --danger-soft: #fde7ee; --info: #007185; /* Amazon clickable teal (same as focus) */ --info-soft: #e0f2f5; + --purple: #7c3aed; /* SD campaign-type badge */ + --purple-soft: #f3e8ff; /* Typography — Premium font stack */ --font-display: 'Geist', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; @@ -147,8 +149,16 @@ color-scheme: light; } -/* Reset */ -*, *::before, *::after { box-sizing: border-box; margin: 0; padding: 0; } +/* Reset + * NOTE: padding is intentionally NOT zeroed on `*`. Astryx components ship + * their own padding as StyleX classes inside `@layer astryx-base`/ + * `@layer astryx-theme`; per the CSS cascade-layers spec, ANY unlayered + * declaration beats a layered one regardless of specificity. A blanket + * `* { padding: 0 }` here (unlayered) was silently zeroing out every + * Astryx `padding` prop (Card, etc.) sitewide. Reset padding only on the + * couple of native elements that actually need it. */ +*, *::before, *::after { box-sizing: border-box; margin: 0; } +ul, ol { padding: 0; } html { font-family: var(--font-body); @@ -1072,7 +1082,7 @@ code, pre { .split { display: grid; - grid-template-columns: 2fr 1fr; + grid-template-columns: minmax(0, 2fr) minmax(0, 1fr); gap: var(--space-5); } @@ -1243,8 +1253,8 @@ textarea.input { } .pill.purple { - background: #f3e8ff; - color: #7c3aed; + background: var(--purple-soft); + color: var(--purple); } .pill.green { @@ -3136,6 +3146,7 @@ textarea.input { } .text-accent { color: var(--landing-accent); } +.object-cover { object-fit: cover; } /* Navigation */ .landing-nav { diff --git a/src/app/page.tsx b/src/app/page.tsx index d7503de..01c873a 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -8,27 +8,27 @@ import { useEffect, useRef, useState } from 'react'; const FEATURES = [ { title: 'Campaign Creation Wizard', - description: 'Build SP, SB, and SD campaigns step by step ΓÇö the same interface you\'ll use in production.', + description: 'Build SP, SB, and SD campaigns step by step — the same interface you\'ll use in production.', icon: ( - + ), }, { title: '7-Day Performance Simulation', - description: 'Watch realistic metrics build ΓÇö ROAS, ACOS, CPC, impressions ΓÇö exactly like a live campaign.', + description: 'Watch realistic metrics build — ROAS, ACOS, CPC, impressions — exactly like a live campaign.', icon: ( - + ), }, { title: 'Search Term Mining', - description: 'Identify which search terms convert. Harvest winners, negate losers ΓÇö the optimization loop that actually moves ACOS.', + description: 'Identify which search terms convert. Harvest winners, negate losers — the optimization loop that actually moves ACOS.', icon: ( - + ), @@ -37,7 +37,7 @@ const FEATURES = [ title: 'Guided Drills', description: 'Click-by-click coaching walks you through the console. Track mistakes, earn scores, level up.', icon: ( - + ), @@ -46,16 +46,16 @@ const FEATURES = [ title: 'Scenario Missions', description: 'Real-world challenges from beginner ACOS reduction to advanced auto-targeting. Get scored, get better.', icon: ( - + ), }, { title: 'Bulk CSV Operations', - description: 'Paste your Amazon bulk export. Validate it, preview the changes, apply it ΓÇö no more spreadsheet errors.', + description: 'Paste your Amazon bulk export. Validate it, preview the changes, apply it — no more spreadsheet errors.', icon: ( - + ), @@ -66,7 +66,7 @@ const STEPS = [ { num: '01', title: 'Build your first campaign', - description: 'Choose SP, SB, or SD. Walk through targeting, bidding, and creative settings ΓÇö exactly like the real console.', + description: 'Choose SP, SB, or SD. Walk through targeting, bidding, and creative settings — exactly like the real console.', }, { num: '02', @@ -141,7 +141,7 @@ export default function LandingPage() { transition={{ duration: 0.6, ease: [0.16, 1, 0.3, 1] }} className="landing-hero-label" > - + No Amazon account needed @@ -175,7 +175,7 @@ export default function LandingPage() { > Start Training Free - + @@ -195,14 +195,14 @@ export default function LandingPage() {
- {/* Problem ΓåÆ Solution */} + {/* Problem → Solution */}
The old way
    -
  • Practice on live campaigns ΓÇö real money, real risk
    • +
  • Practice on live campaigns — real money, real risk
  • Learn by trial and expensive error
  • No safe space to experiment with new strategies
  • No guided coaching when you get stuck
    • @@ -267,7 +267,7 @@ export default function LandingPage() {

    The exact interface you'll use in production

    -

    No simplified training wheels ΓÇö this is the real console experience.

    +

    No simplified training wheels — this is the real console experience.

     @@ -298,7 +298,7 @@ export default function LandingPage() {

    Build your first campaign in the next 5 minutes.

    Open the Simulator - + diff --git a/src/components/AdConsole/AdConsole.tsx b/src/components/AdConsole/AdConsole.tsx index dd8d1f5..84b4247 100644 --- a/src/components/AdConsole/AdConsole.tsx +++ b/src/components/AdConsole/AdConsole.tsx @@ -4,6 +4,7 @@ import { Button } from '@astryxdesign/core/Button'; import { useAdConsoleStore } from '@/engine/ad-console/store'; import { Topbar } from './layout/Topbar'; +import { Sidebar } from './layout/Sidebar'; import { ErrorBoundary } from './ErrorBoundary'; import { Dashboard } from './Dashboard'; import { CampaignManager } from './CampaignManager'; @@ -50,11 +51,14 @@ export function AdConsole() {
    Skip to main content -
    - -
    {renderView()}
    -     -
    +
    + +
    + +
    {renderView()}
    +     +
    +
    ); } diff --git a/src/components/AdConsole/CampaignManager.tsx b/src/components/AdConsole/CampaignManager.tsx index 9666c2f..5dd8392 100644 --- a/src/components/AdConsole/CampaignManager.tsx +++ b/src/components/AdConsole/CampaignManager.tsx @@ -5,7 +5,7 @@ import { Button } from '@astryxdesign/core/Button'; import { Card } from '@astryxdesign/core/Card'; import { useCampaignManager } from './hooks/useCampaignManager'; import { MetricCard } from './metrics/MetricCard'; -import { calc, formatMoney, formatWhole, formatPercent, formatBid, formatRoas, acosClass } from '@/engine/ad-console/core/engine'; +import { calc, totalMetrics, formatMoney, formatWhole, formatPercent, formatBid, formatRoas, acosClass } from '@/engine/ad-console/core/engine'; import type { FilterState } from '@/engine/ad-console/types'; import { ManagerCampaignsTab } from './details/ManagerCampaignsTab'; import { ManagerAdGroupsTab } from './details/ManagerAdGroupsTab'; @@ -15,11 +15,13 @@ import { ManagerNegativesTab } from './details/ManagerNegativesTab'; export function CampaignManager() { const { - filteredCampaigns, filter, selectedTab, portfolioOptions, + campaigns, filteredCampaigns, filter, selectedTab, portfolioOptions, setFilter, selectCampaign, setTab, toggleCampaignStatus, duplicateCampaign, archiveCampaign, runSimulation, setView, } = useCampaignManager(); + const clearFilters = () => setFilter({ type: 'All', status: 'All', portfolio: 'All', search: '' }); + const [simulating, setSimulating] = useState(false); const handleSimulate = async () => { @@ -84,23 +86,13 @@ export function CampaignManager() { -
    {(() => { - const m = filteredCampaigns.reduce( - (acc, c) => { - acc.impressions += c.metrics.impressions; - acc.clicks += c.metrics.clicks; - acc.spend += c.metrics.spend; - acc.sales += c.metrics.sales; - acc.orders += c.metrics.orders; - return acc; - }, - { impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, - ); + const m = totalMetrics(filteredCampaigns); const x = calc(m); const acosTone = x.acos <= 0 ? '' : x.acos <= 30 ? 'good' : 'bad'; return ( @@ -129,11 +121,13 @@ export function CampaignManager() { {selectedTab === 'campaigns' && ( 0} onSelect={selectCampaign} onToggleStatus={toggleCampaignStatus} onDuplicate={duplicateCampaign} onArchive={archiveCampaign} onCreate={() => setView('create')} + onClearFilters={clearFilters} /> )} {selectedTab === 'adgroups' && } diff --git a/src/components/AdConsole/Dashboard.tsx b/src/components/AdConsole/Dashboard.tsx index 75c0bd7..d79c90d 100644 --- a/src/components/AdConsole/Dashboard.tsx +++ b/src/components/AdConsole/Dashboard.tsx @@ -1,5 +1,6 @@ 'use client'; +import { useMemo } from 'react'; import { Button } from '@astryxdesign/core/Button'; import { Table } from '@astryxdesign/core/Table'; import { Card } from '@astryxdesign/core/Card'; @@ -15,16 +16,23 @@ export function Dashboard() { const selectCampaign = useAdConsoleStore((s) => s.selectCampaign); const totalMetrics = useAdConsoleStore((s) => s.totalMetricsCalc); - const m = totalMetrics(); - const d = calc(m); - const tiles = getKpiTiles({ - impressions: m.impressions, - clicks: m.clicks, - spend: m.spend, - sales: m.sales, - orders: m.orders, - units: m.orders, - }); + // state.campaigns keeps the same array reference for any state change + // that doesn't touch campaigns (filter/tab/mobile-menu toggles, etc.), + // so this skips recomputing the full-campaign-list aggregate on those. + const m = useMemo(() => totalMetrics(), [state.campaigns]); // eslint-disable-line react-hooks/exhaustive-deps + const d = useMemo(() => calc(m), [m]); + const tiles = useMemo( + () => + getKpiTiles({ + impressions: m.impressions, + clicks: m.clicks, + spend: m.spend, + sales: m.sales, + orders: m.orders, + units: m.orders, + }), + [m], + ); const enabledCount = state.campaigns.filter((c) => c.status === 'Enabled').length; const acosHealthy = d.acos > 0 && d.acos <= 30; @@ -65,13 +73,13 @@ export function Dashboard() {
    - +

    Campaigns

    {enabledCount} enabled · {state.campaigns.length} total
    {renderCampaignTable(state.campaigns.slice(0, 8), selectCampaign, calc, setView)} - +
    @@ -114,16 +122,6 @@ export function Dashboard() { ); } -function fmtMoney(n: number) { - return '$' + n.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 }); -} -function fmtWhole(n: number) { - return n.toLocaleString(); -} -function fmtPercent(n: number) { - return n.toFixed(2) + '%'; -} - type Tone = '' | 'good' | 'bad'; function kpiDelta( @@ -219,15 +217,15 @@ function renderCampaignTable( {c.status} - {fmtMoney(c.dailyBudget)} + {formatMoney(c.dailyBudget)} {c.targetingMode} - {fmtWhole(c.metrics.impressions)} - {fmtWhole(c.metrics.clicks)} - {fmtMoney(c.metrics.spend)} - {fmtMoney(c.metrics.sales)} - {fmtPercent(x.acos)} + {formatWhole(c.metrics.impressions)} + {formatWhole(c.metrics.clicks)} + {formatMoney(c.metrics.spend)} + {formatMoney(c.metrics.sales)} + {formatPercent(x.acos)} ); })} diff --git a/src/components/AdConsole/PortfolioOverview.tsx b/src/components/AdConsole/PortfolioOverview.tsx index 402140f..6ab86af 100644 --- a/src/components/AdConsole/PortfolioOverview.tsx +++ b/src/components/AdConsole/PortfolioOverview.tsx @@ -5,7 +5,7 @@ import { Button } from '@astryxdesign/core/Button'; import { Table } from '@astryxdesign/core/Table'; import { Card } from '@astryxdesign/core/Card'; import { useAdConsoleStore } from '@/engine/ad-console/store'; -import { calc, formatMoney, formatWhole, formatPercent, formatRoas, acosClass } from '@/engine/ad-console/core/engine'; +import { calc, totalMetrics as sumMetrics, formatMoney, formatWhole, formatPercent, formatRoas, acosClass } from '@/engine/ad-console/core/engine'; export function PortfolioOverview() { const state = useAdConsoleStore((s) => s.state); @@ -30,35 +30,11 @@ export function PortfolioOverview() { return Array.from(map.entries()).map(([name, camps]) => ({ name, campaigns: camps, - metrics: camps.reduce( - (acc, c) => { - acc.impressions += c.metrics.impressions; - acc.clicks += c.metrics.clicks; - acc.spend += c.metrics.spend; - acc.sales += c.metrics.sales; - acc.orders += c.metrics.orders; - return acc; - }, - { impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, - ), + metrics: sumMetrics(camps), })); }, [state.campaigns]); - const totalMetrics = useMemo( - () => - state.campaigns.reduce( - (acc, c) => { - acc.impressions += c.metrics.impressions; - acc.clicks += c.metrics.clicks; - acc.spend += c.metrics.spend; - acc.sales += c.metrics.sales; - acc.orders += c.metrics.orders; - return acc; - }, - { impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, - ), - [state.campaigns], - ); + const totalMetrics = useMemo(() => sumMetrics(state.campaigns), [state.campaigns]); const totalDerived = calc(totalMetrics); @@ -119,36 +95,38 @@ export function PortfolioOverview() { portfolios.map((pf) => { const x = calc(pf.metrics); return ( - -
    - {manageMode ? ( -
    - - setRenameMap((m) => ({ ...m, [pf.name]: e.target.value }))} - onBlur={(e) => { - const v = e.target.value.trim(); - if (v && v !== pf.name) renamePortfolio(pf.name, v); +
    + +
    + {manageMode ? ( +
    + + setRenameMap((m) => ({ ...m, [pf.name]: e.target.value }))} + onBlur={(e) => { + const v = e.target.value.trim(); + if (v && v !== pf.name) renamePortfolio(pf.name, v); + }} /> + {pf.campaigns.length} campaign{pf.campaigns.length !== 1 ? 's' : ''} +
    - ) : ( - <> -

    {pf.name}

    - {pf.campaigns.length} campaign{pf.campaigns.length !== 1 ? 's' : ''} - - )} -
    -
    -
    Spend
    {formatMoney(pf.metrics.spend)}
    -
    Sales
    {formatMoney(pf.metrics.sales)}
    -
    {formatPercent(x.acos)}
    ACOS
    -
    {formatRoas(x.roas)}
    ROAS
    -
    +
    + ) : ( + <> +

    {pf.name}

    + {pf.campaigns.length} campaign{pf.campaigns.length !== 1 ? 's' : ''} + + )} +
    +
    +
    Spend
    {formatMoney(pf.metrics.spend)}
    +
    Sales
    {formatMoney(pf.metrics.sales)}
    +
    {formatPercent(x.acos)}
    ACOS
    +
    {formatRoas(x.roas)}
    ROAS
    +
    + @@ -162,8 +140,8 @@ export function PortfolioOverview() { return ( @@ -191,7 +169,7 @@ export function PortfolioOverview() { })}
    -
    - +
    ); }) )} diff --git a/src/components/AdConsole/__tests__/ManagerCampaignsTab.test.tsx b/src/components/AdConsole/__tests__/ManagerCampaignsTab.test.tsx index 49500d3..525bc7d 100644 --- a/src/components/AdConsole/__tests__/ManagerCampaignsTab.test.tsx +++ b/src/components/AdConsole/__tests__/ManagerCampaignsTab.test.tsx @@ -79,6 +79,8 @@ describe('ManagerCampaignsTab — column alignment (H-02)', () => { onDuplicate={noop} onArchive={noop} onCreate={noop} + onClearFilters={noop} + hasAnyCampaigns={true} />, ); @@ -106,7 +108,7 @@ describe('ManagerCampaignsTab — column alignment (H-02)', () => { expect(cellText(13)).toBe('8.00x'); // ROAS }); - it('shows the empty state when there are no campaigns', () => { + it('shows the "create your first campaign" empty state when there are no campaigns at all', () => { render( { onDuplicate={noop} onArchive={noop} onCreate={noop} + onClearFilters={noop} + hasAnyCampaigns={false} />, ); expect(screen.getByText('No campaigns yet')).toBeDefined(); }); + it('shows a "no matches" empty state (not "create your first campaign") when filters yield zero results', () => { + render( + , + ); + expect(screen.getByText('No campaigns match your filters')).toBeDefined(); + expect(screen.queryByText('No campaigns yet')).toBeNull(); + expect(screen.getByText('Clear filters')).toBeDefined(); + }); + it('renders one row per campaign', () => { const a = { ...FIXTURE_CAMPAIGN, id: 'a', name: 'Alpha' }; const b = { ...FIXTURE_CAMPAIGN, id: 'b', name: 'Bravo' }; @@ -131,6 +153,8 @@ describe('ManagerCampaignsTab — column alignment (H-02)', () => { onDuplicate={noop} onArchive={noop} onCreate={noop} + onClearFilters={noop} + hasAnyCampaigns={true} />, ); const rows = container.querySelectorAll('tbody tr'); diff --git a/src/components/AdConsole/__tests__/contracts/cards-astryx.test.tsx b/src/components/AdConsole/__tests__/contracts/cards-astryx.test.tsx index 5dcbad0..d7e2f7e 100644 --- a/src/components/AdConsole/__tests__/contracts/cards-astryx.test.tsx +++ b/src/components/AdConsole/__tests__/contracts/cards-astryx.test.tsx @@ -44,8 +44,10 @@ describe('Astryx Card contract — presence and variant wiring', () => { const c = useAdConsoleStore.getState().state.campaigns[0]!; render(); const astryxCards = document.querySelectorAll('.astryx-card'); - // OverviewTab has 3 .card.pad sections (Settings, Products, Top targets) - expect(astryxCards.length).toBeGreaterThanOrEqual(3); + // OverviewTab has 2 Card sections (Settings, Products). "Top targets" + // is a dense table and is deliberately NOT Card-wrapped, per the + // "Table edge-to-edge, never Card-wrapped" convention. + expect(astryxCards.length).toBeGreaterThanOrEqual(2); }); it('Astrox cards have data-variant attribute', () => { diff --git a/src/components/AdConsole/__tests__/contracts/sidebar.test.tsx b/src/components/AdConsole/__tests__/contracts/sidebar.test.tsx new file mode 100644 index 0000000..2312608 --- /dev/null +++ b/src/components/AdConsole/__tests__/contracts/sidebar.test.tsx @@ -0,0 +1,88 @@ +/** + * Sidebar contract tests. + * + * Pins the fix for a real navigation gap: `Sidebar` (nav/consoleNav's + * `getLeftRail`/training rail) existed with full unit test coverage but was + * never mounted into `AdConsole`, so Missions/Reports/Bulk ops/Trainer/ + * Integrity were unreachable from the desktop UI. This locks in that the + * sidebar renders and actually drives navigation for those views. + */ +import React from 'react'; +import { describe, it, expect, beforeEach } from 'vitest'; +import { render, fireEvent } from '@testing-library/react'; +import { SessionProvider } from 'next-auth/react'; +import { AdConsole } from '../../AdConsole'; +import { useAdConsoleStore } from '@/engine/ad-console/store'; + +function resetStore() { + useAdConsoleStore.getState().resetAll(); +} + +function renderAdConsole() { + return render( + + + , + ); +} + +describe('Sidebar contract — mounted and reachable', () => { + beforeEach(resetStore); + + it('renders a nav.app-sidebar inside app-body', () => { + renderAdConsole(); + const sidebar = document.querySelector('.app-body > nav.app-sidebar'); + expect(sidebar).not.toBeNull(); + }); + + it('shows the training rail (Missions, Reports, Bulk ops, Trainer, Integrity) when on a training view', () => { + useAdConsoleStore.getState().setView('drills'); + renderAdConsole(); + + const sidebar = document.querySelector('.app-sidebar')!; + for (const label of ['Drills', 'Missions', 'Reports', 'Bulk ops', 'Trainer', 'Integrity']) { + const btn = Array.from(sidebar.querySelectorAll('button.sidebar-item')).find( + (b) => b.textContent?.trim() === label, + ); + expect(btn, `sidebar item "${label}" must exist`).not.toBeUndefined(); + } + }); + + it('clicking "Missions" in the sidebar navigates to the missions view', () => { + useAdConsoleStore.getState().setView('drills'); + renderAdConsole(); + + const sidebar = document.querySelector('.app-sidebar')!; + const btn = Array.from(sidebar.querySelectorAll('button.sidebar-item')).find( + (b) => b.textContent?.trim() === 'Missions', + ); + expect(btn).not.toBeUndefined(); + fireEvent.click(btn!); + + expect(useAdConsoleStore.getState().view).toBe('missions'); + }); + + it('clicking "Reports" in the sidebar navigates to the reports view', () => { + useAdConsoleStore.getState().setView('trainer'); + renderAdConsole(); + + const sidebar = document.querySelector('.app-sidebar')!; + const btn = Array.from(sidebar.querySelectorAll('button.sidebar-item')).find( + (b) => b.textContent?.trim() === 'Reports', + ); + expect(btn).not.toBeUndefined(); + fireEvent.click(btn!); + + expect(useAdConsoleStore.getState().view).toBe('reports'); + }); + + it('marks the active view\'s rail item with aria-current="page"', () => { + useAdConsoleStore.getState().setView('integrity'); + renderAdConsole(); + + const sidebar = document.querySelector('.app-sidebar')!; + const active = sidebar.querySelector('button.sidebar-item[aria-current="page"]'); + expect(active).not.toBeNull(); + expect(active?.textContent?.trim()).toBe('Integrity'); + }); +}); diff --git a/src/components/AdConsole/details/AdGroupsTab.tsx b/src/components/AdConsole/details/AdGroupsTab.tsx index 4283c02..1423be9 100644 --- a/src/components/AdConsole/details/AdGroupsTab.tsx +++ b/src/components/AdConsole/details/AdGroupsTab.tsx @@ -10,6 +10,7 @@ import { TextInput } from '@astryxdesign/core/TextInput'; import type { Campaign } from '@/engine/ad-console/types'; import { useAdConsoleStore } from '@/engine/ad-console/store'; import { calc, formatMoney, formatWhole, formatPercent, formatBid, acosClass } from '@/engine/ad-console/core/engine'; +import { MIN_BID } from '@/lib/validation'; import { EmptyState } from './EmptyState'; interface Props { @@ -62,7 +63,10 @@ export function AdGroupsTab({ campaign: c }: Props) { />
    )} - {((c.type === 'SB' || c.type === 'SD') && ((c.type === 'SB' && c.adFormat === 'Video') || (c.type === 'SD' && c.adFormat === 'Video creative'))) && c.creative?.video && ( -
    + {isVideoFormat(c.type, c.adFormat) && c.creative?.video && ( +
    ▶ Video: {c.creative.video}
    @@ -102,7 +102,7 @@ export function OverviewTab({ campaign: c }: Props) {
    )}     - +

    Top targets by profit signal

    Use to train bid optimization
    @@ -126,7 +126,7 @@ export function OverviewTab({ campaign: c }: Props) { })}
    TargetBidImpr.ClicksCPCSpendSalesOrdersACOSROAS
    - +
    ); } diff --git a/src/components/AdConsole/details/TargetsTab.tsx b/src/components/AdConsole/details/TargetsTab.tsx index e7c4206..518eea4 100644 --- a/src/components/AdConsole/details/TargetsTab.tsx +++ b/src/components/AdConsole/details/TargetsTab.tsx @@ -7,6 +7,7 @@ import { Card } from '@astryxdesign/core/Card'; import type { Campaign } from '@/engine/ad-console/types'; import { useAdConsoleStore } from '@/engine/ad-console/store'; import { calc, formatMoney, formatWhole, formatPercent, formatBid, formatRoas, acosClass } from '@/engine/ad-console/core/engine'; +import { MIN_BID } from '@/lib/validation'; import { EmptyState } from './EmptyState'; interface Props { @@ -45,8 +46,8 @@ export function TargetsTab({ campaign: c }: Props) { )} {showAddKeywordForm && ( - -
    + +
    setNewKeywordValue(e.target.value)} placeholder="Enter keyword" /> @@ -68,7 +69,7 @@ export function TargetsTab({ campaign: c }: Props) {
    -
    +
    - {selected.rows.length > 0 && ( + {selected.rows.length > 0 ? ( {Object.keys(selected.rows[0]).map((h) => )} @@ -93,8 +94,10 @@ export function ReportsPage() { })}
    {h}
    + ) : ( + )} - +
    )} {!requests.length && ( diff --git a/src/components/AdConsole/mobile/MobileNav.tsx b/src/components/AdConsole/mobile/MobileNav.tsx index fd660ae..d64d174 100644 --- a/src/components/AdConsole/mobile/MobileNav.tsx +++ b/src/components/AdConsole/mobile/MobileNav.tsx @@ -3,12 +3,13 @@ import { useEffect } from 'react'; import { useAdConsoleStore } from '@/engine/ad-console/store'; import { useBreakpoint } from '@/lib/useBreakpoint'; -import { getLeftRail, type NavView } from '../nav/consoleNav'; +import { getLeftRail, isSidebarItemActive, resolveSidebarClick, sidebarSectionForView } from '../nav/consoleNav'; const GROUP_TITLES: Record = { campaigns: 'Campaign Manager', portfolios: 'Portfolios', measurement: 'Measurement', + training: 'Training', }; export function MobileNav() { @@ -19,11 +20,13 @@ export function MobileNav() { const toggleMobileMenu = useAdConsoleStore((s) => s.toggleMobileMenu); const closeMobileMenu = useAdConsoleStore((s) => s.closeMobileMenu); const view = useAdConsoleStore((s) => s.view); + const selectedTab = useAdConsoleStore((s) => s.state.selectedTab); const setView = useAdConsoleStore((s) => s.setView); + const setTab = useAdConsoleStore((s) => s.setTab); const runSimulation = useAdConsoleStore((s) => s.runSimulation); const resetAll = useAdConsoleStore((s) => s.resetAll); - const section: NavView = view === 'portfolio' ? 'portfolio' : 'campaigns'; + const section = sidebarSectionForView(view); const items = getLeftRail(section); const groups: Record = {}; @@ -79,9 +82,12 @@ export function MobileNav() { {groupItems.map((item) => (
    {!d.name.trim() &&
    Campaign name is required before launch.
    }     diff --git a/src/components/AdConsole/wizard/__tests__/Step6ReviewLaunch.test.tsx b/src/components/AdConsole/wizard/__tests__/Step6ReviewLaunch.test.tsx new file mode 100644 index 0000000..6e25da7 --- /dev/null +++ b/src/components/AdConsole/wizard/__tests__/Step6ReviewLaunch.test.tsx @@ -0,0 +1,79 @@ +/** + * Step6ReviewLaunch pins a real bug found during a wizard walkthrough: + * `audienceLookback` defaults to '30' in makeDraft() regardless of + * campaign type, so the old `{d.audienceLookback && ...}` condition + * showed a "Lookback" row on every campaign's review step — including + * Sponsored Products, which has no such concept. The Lookback row should + * only appear when targetingMode is actually an audience mode. + */ +import { describe, it, expect, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { Step6ReviewLaunch } from '../Step6ReviewLaunch'; +import { useAdConsoleStore } from '@/engine/ad-console/store'; + +function resetStore() { + useAdConsoleStore.getState().resetAll(); + useAdConsoleStore.getState().resetDraft(); +} + +describe('Step6ReviewLaunch — Lookback row', () => { + beforeEach(resetStore); + + it('does not show Lookback for a default (non-audience) SP draft', () => { + render(); + expect(screen.queryByText('Lookback')).toBeNull(); + }); + + it('shows Lookback once targetingMode is an audience mode', () => { + useAdConsoleStore.getState().updateDraft('type', 'SD'); + useAdConsoleStore.getState().updateDraft('targetingMode', 'Audiences - views remarketing'); + useAdConsoleStore.getState().updateDraft('audienceLookback', '60'); + render(); + expect(screen.getByText('Lookback')).toBeTruthy(); + expect(screen.getByText('60 days')).toBeTruthy(); + }); + + it('hides Lookback again if targetingMode is switched back to a non-audience mode', () => { + useAdConsoleStore.getState().updateDraft('type', 'SB'); + useAdConsoleStore.getState().updateDraft('targetingMode', 'Keyword'); + render(); + expect(screen.queryByText('Lookback')).toBeNull(); + }); +}); + +describe('Step6ReviewLaunch — targeting/creative summary rows', () => { + beforeEach(resetStore); + + it('shows ASIN and category target counts when entered', () => { + useAdConsoleStore.getState().updateDraft('asinTargets', 'B0ABC123\nB0DEF456'); + useAdConsoleStore.getState().updateDraft('categoryTargets', 'Drinkware'); + render(); + expect(screen.getByText('ASIN targets')).toBeTruthy(); + expect(screen.getByText('2 entered')).toBeTruthy(); + expect(screen.getByText('Category targets')).toBeTruthy(); + expect(screen.getByText('1 entered')).toBeTruthy(); + }); + + it.each(['SB', 'SD'] as const)('shows the %s creative headline, brand, and destination when set', (type) => { + useAdConsoleStore.getState().updateDraft('type', type); + useAdConsoleStore.getState().updateDraft('creative', { + headline: 'Discover your perfect brew', + brandName: 'Acme Coffee', + destination: 'Brand Store', + }); + render(); + expect(screen.getByText('Headline')).toBeTruthy(); + expect(screen.getByText('Discover your perfect brew')).toBeTruthy(); + expect(screen.getByText('Brand')).toBeTruthy(); + expect(screen.getByText('Acme Coffee')).toBeTruthy(); + expect(screen.getByText('Destination')).toBeTruthy(); + expect(screen.getByText('Brand Store')).toBeTruthy(); + }); + + it('does not show creative rows for a fresh SP draft with no creative set', () => { + render(); + expect(screen.queryByText('Headline')).toBeNull(); + expect(screen.queryByText('Brand')).toBeNull(); + expect(screen.queryByText('Destination')).toBeNull(); + }); +}); diff --git a/src/engine/ad-console/__tests__/store.test.ts b/src/engine/ad-console/__tests__/store.test.ts index d3226ac..d251bb1 100644 --- a/src/engine/ad-console/__tests__/store.test.ts +++ b/src/engine/ad-console/__tests__/store.test.ts @@ -26,6 +26,29 @@ describe('Store actions', () => { expect(campaign?.targets.map(t => t.match)).toEqual(['Exact', 'Phrase', 'Broad']); }); + it('assigns unique ids even when launched within the same millisecond', () => { + // Regression: the id used to be built from Date.now() alone with no + // counter, so two launches in the same tick (e.g. a double-clicked + // "Launch" button) produced identical campaign ids. + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1700000000000); + try { + const store = useAdConsoleStore.getState(); + store.updateDraft('name', 'First'); + store.launchCampaign(); + store.updateDraft('name', 'Second'); + store.launchCampaign(); + + const state = useAdConsoleStore.getState().state; + const first = state.campaigns.find(c => c.name === 'First'); + const second = state.campaigns.find(c => c.name === 'Second'); + expect(first?.id).toBeDefined(); + expect(second?.id).toBeDefined(); + expect(first?.id).not.toBe(second?.id); + } finally { + nowSpy.mockRestore(); + } + }); + it('does not create campaign if name is empty', () => { const store = useAdConsoleStore.getState(); store.updateDraft('name', ''); diff --git a/src/engine/ad-console/core/__tests__/adgroup.test.ts b/src/engine/ad-console/core/__tests__/adgroup.test.ts index 9ba39e0..154f2fa 100644 --- a/src/engine/ad-console/core/__tests__/adgroup.test.ts +++ b/src/engine/ad-console/core/__tests__/adgroup.test.ts @@ -97,7 +97,7 @@ describe('setAdGroupStatus', () => { }); describe('setAdGroupDefaultBid', () => { - it('sets the default bid (clamped to minimum)', () => { + it('sets the default bid', () => { const next = setAdGroupDefaultBid(makeCampaign(), 'AG1', 1.25); expect(next.adGroups[0]!.defaultBid).toBe(1.25); }); @@ -105,6 +105,18 @@ describe('setAdGroupDefaultBid', () => { it('fails fast on unknown ad group id', () => { expect(() => setAdGroupDefaultBid(makeCampaign(), 'NOPE', 1)).toThrow(); }); + + it('fails fast on a bid below the real minimum instead of silently substituting it', () => { + // Previously: 0 and 0.01 passed the non-negative check and were then + // silently rewritten to $0.02, discarding what the caller asked for. + expect(() => setAdGroupDefaultBid(makeCampaign(), 'AG1', 0)).toThrow(); + expect(() => setAdGroupDefaultBid(makeCampaign(), 'AG1', 0.01)).toThrow(); + }); + + it('fails fast on a negative or NaN default bid', () => { + expect(() => setAdGroupDefaultBid(makeCampaign(), 'AG1', -1)).toThrow(); + expect(() => setAdGroupDefaultBid(makeCampaign(), 'AG1', NaN)).toThrow(); + }); }); describe('removeAdGroup', () => { diff --git a/src/engine/ad-console/core/__tests__/engine.test.ts b/src/engine/ad-console/core/__tests__/engine.test.ts index 3e9bb92..1501c60 100644 --- a/src/engine/ad-console/core/__tests__/engine.test.ts +++ b/src/engine/ad-console/core/__tests__/engine.test.ts @@ -27,6 +27,7 @@ import { campaignById, totalMetrics, normalizeCampaign, + isVideoFormat, } from '../engine'; import type { Campaign, CampaignType, CampaignStatus, Metrics, Target, SearchTerm, Negative } from '../types'; @@ -195,6 +196,12 @@ describe('target operations', () => { expect(adjustTargetBid(c, 'T1', 1.5).targets[0]!.bid).toBe(1.5); }); + it('floors a decrement at the platform minimum instead of throwing (the "-10%" button on an already-cheap bid)', () => { + const c = makeCampaign({ targets: [{ id: 'T1', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'kw', match: 'Exact', bid: 0.02, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }] }); + expect(() => adjustTargetBid(c, 'T1', 0.9)).not.toThrow(); + expect(adjustTargetBid(c, 'T1', 0.9).targets[0]!.bid).toBe(0.02); + }); + it('fails fast on a NaN bid instead of silently storing NaN', () => { const c = makeCampaign({ targets: [{ id: 'T1', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'kw', match: 'Exact', bid: 0.75, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }] }); expect(() => setTargetBid(c, 'T1', NaN)).toThrow(); @@ -205,6 +212,12 @@ describe('target operations', () => { expect(() => setTargetBid(c, 'T1', -5)).toThrow(); }); + it('fails fast on a bid below the real minimum instead of silently substituting it', () => { + const c = makeCampaign({ targets: [{ id: 'T1', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'kw', match: 'Exact', bid: 0.75, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }] }); + expect(() => setTargetBid(c, 'T1', 0)).toThrow(); + expect(() => setTargetBid(c, 'T1', 0.01)).toThrow(); + }); + it('pauses then re-enables a target', () => { const c = makeCampaign({ targets: [{ id: 'T1', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'kw', match: 'Exact', bid: 0.75, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }] }); expect(pauseTarget(c, 'T1').targets[0]!.status).toBe('Paused'); @@ -328,4 +341,29 @@ describe('campaign creation flow', () => { expect(campaign.targets[0].value).toBe('coffee filter'); expect(campaign.targets[1].value).toBe('coffee maker'); }); +}); + +describe('isVideoFormat', () => { + it('is true for an SB campaign with adFormat "Video"', () => { + expect(isVideoFormat('SB', 'Video')).toBe(true); + }); + + it('is false for an SB campaign with any other adFormat', () => { + expect(isVideoFormat('SB', 'Product collection')).toBe(false); + expect(isVideoFormat('SB', undefined)).toBe(false); + }); + + it('is true for an SD campaign with adFormat "Video creative"', () => { + expect(isVideoFormat('SD', 'Video creative')).toBe(true); + }); + + it('is false for an SD campaign with any other adFormat', () => { + expect(isVideoFormat('SD', 'Video')).toBe(false); + expect(isVideoFormat('SD', undefined)).toBe(false); + }); + + it('is false for SP regardless of adFormat', () => { + expect(isVideoFormat('SP', 'Video')).toBe(false); + expect(isVideoFormat('SP', undefined)).toBe(false); + }); }); \ No newline at end of file diff --git a/src/engine/ad-console/core/__tests__/simulation.test.ts b/src/engine/ad-console/core/__tests__/simulation.test.ts index 6a2a04b..912b3bb 100644 --- a/src/engine/ad-console/core/__tests__/simulation.test.ts +++ b/src/engine/ad-console/core/__tests__/simulation.test.ts @@ -171,6 +171,24 @@ describe('simulateDays', () => { expect(terms.length).toBe(unique.size); }); + it('does not re-add duplicate search terms across repeated simulateDays runs', () => { + const c = makeCampaign({ + targets: [ + { id: 'T1', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'coffee', match: 'Broad', bid: 1, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, + ], + }); + const [firstRun] = simulateDays([c], 7); + const [secondRun] = simulateDays([firstRun], 7); + const firstTerms = firstRun.searchTerms.map(st => st.term); + const terms = secondRun.searchTerms.map(st => st.term); + expect(firstTerms.length).toBeGreaterThan(0); + expect(terms).toEqual(expect.arrayContaining(firstTerms)); + const unique = new Set(terms); + expect(terms.length).toBe(unique.size); + // The second run should carry forward the first run's terms, not duplicate them. + expect(secondRun.searchTerms.length).toBeGreaterThanOrEqual(firstRun.searchTerms.length); + }); + it('fails fast on a negative days value instead of corrupting metrics', () => { const c = makeCampaign(); expect(() => simulateDays([c], -7)).toThrow(); diff --git a/src/engine/ad-console/core/engine/adgroup.ts b/src/engine/ad-console/core/engine/adgroup.ts index c070462..a971bc1 100644 --- a/src/engine/ad-console/core/engine/adgroup.ts +++ b/src/engine/ad-console/core/engine/adgroup.ts @@ -2,7 +2,7 @@ * Ad group CRUD operations. */ import type { Campaign, CampaignStatus, AdGroup, ProductAd, Ad } from '../types'; -import { assertNonEmpty, assertFiniteNonNegative, ValidationError } from '../../../../lib/validation'; +import { assertNonEmpty, assertValidBid, ValidationError } from '../../../../lib/validation'; import { generateId } from './id'; export function addAdGroup(c: Campaign, name: string, defaultBid?: number): Campaign { @@ -82,15 +82,15 @@ export function setAdGroupStatus(c: Campaign, adGroupId: string, status: Campaig } export function setAdGroupDefaultBid(c: Campaign, adGroupId: string, defaultBid: number): Campaign { - assertFiniteNonNegative('default bid', defaultBid); + assertValidBid('default bid', defaultBid); const ag = c.adGroups.find((a) => a.id === adGroupId); if (!ag) throw new ValidationError(`Unknown ad group: ${adGroupId}`); return { ...c, adGroups: c.adGroups.map((a) => - a.id === adGroupId ? { ...a, defaultBid: Math.max(0.02, defaultBid) } : a, + a.id === adGroupId ? { ...a, defaultBid } : a, ), - history: [...c.history, `Ad group "${ag.name}" default bid -> $${Math.max(0.02, defaultBid).toFixed(2)}`], + history: [...c.history, `Ad group "${ag.name}" default bid -> $${defaultBid.toFixed(2)}`], }; } diff --git a/src/engine/ad-console/core/engine/campaign.ts b/src/engine/ad-console/core/engine/campaign.ts index 5cd7bae..301b264 100644 --- a/src/engine/ad-console/core/engine/campaign.ts +++ b/src/engine/ad-console/core/engine/campaign.ts @@ -306,4 +306,14 @@ export function savePlacements( placements, history: [...c.history, changes.length ? `Placements updated: ${changes.join(', ')}` : 'Placements saved (no changes)'], }; +} + +/** + * Single source of truth for which adFormat value means "video" for a + * given campaign type — SB and SD each use a different string for it. + */ +export function isVideoFormat(type: CampaignType, adFormat: string | undefined): boolean { + if (type === 'SB') return adFormat === 'Video'; + if (type === 'SD') return adFormat === 'Video creative'; + return false; } \ No newline at end of file diff --git a/src/engine/ad-console/core/engine/index.ts b/src/engine/ad-console/core/engine/index.ts index 8ba9c66..9499a08 100644 --- a/src/engine/ad-console/core/engine/index.ts +++ b/src/engine/ad-console/core/engine/index.ts @@ -3,7 +3,7 @@ */ export { generateId, resetIdCounter } from './id'; export { calc, totalMetrics, metricDefaults, formatMoney, formatWhole, formatBid, formatPercent, formatRoas, acosClass } from './metrics'; -export { normalizeCampaign, toggleCampaignStatus, archiveCampaign, duplicateCampaign, updateCampaignSettings, savePlacements } from './campaign'; +export { normalizeCampaign, toggleCampaignStatus, archiveCampaign, duplicateCampaign, updateCampaignSettings, savePlacements, isVideoFormat } from './campaign'; export { addTarget, addKeyword, addAutoTarget, addAsinTarget, addCategoryTarget, removeTarget, setTargetBid, adjustTargetBid, pauseTarget, setTargetStatus } from './target'; export { addAdGroup, addProductAd, addAd, renameAdGroup, setAdGroupStatus, setAdGroupDefaultBid, removeAdGroup } from './adgroup'; export { isFilteredByNegative, addNegative, addNegativeKeyword, addNegativeAsin, addNegativeCategory, harvestTerm, getHarvestCandidates, getNegativeCandidates } from './negative'; diff --git a/src/engine/ad-console/core/engine/target.ts b/src/engine/ad-console/core/engine/target.ts index e5b6d6d..39478a3 100644 --- a/src/engine/ad-console/core/engine/target.ts +++ b/src/engine/ad-console/core/engine/target.ts @@ -5,7 +5,7 @@ import type { Campaign, CampaignStatus, MatchType, Target, TargetType } from '../types'; -import { assertNonEmpty, assertFiniteNonNegative, ValidationError } from '../../../../lib/validation'; +import { assertNonEmpty, assertFiniteNonNegative, assertValidBid, MIN_BID, ValidationError } from '../../../../lib/validation'; import { generateId } from './id'; export interface AddTargetOptions { @@ -132,12 +132,12 @@ export function removeTarget(c: Campaign, targetId: string): Campaign { } export function setTargetBid(c: Campaign, targetId: string, newBid: number): Campaign { - assertFiniteNonNegative('bid', newBid); + assertValidBid('bid', newBid); return { ...c, targets: c.targets.map((t) => t.id === targetId - ? { ...t, bid: Math.max(0.02, newBid) } + ? { ...t, bid: newBid } : t, ), history: [ @@ -145,7 +145,7 @@ export function setTargetBid(c: Campaign, targetId: string, newBid: number): Cam (() => { const t = c.targets.find((x) => x.id === targetId); return t - ? `Bid for "${t.value}" (${t.type}) changed from $${t.bid.toFixed(2)} to $${Math.max(0.02, newBid).toFixed(2)}` + ? `Bid for "${t.value}" (${t.type}) changed from $${t.bid.toFixed(2)} to $${newBid.toFixed(2)}` : `Bid updated for target ${targetId}`; })(), ], @@ -155,7 +155,10 @@ export function setTargetBid(c: Campaign, targetId: string, newBid: number): Cam export function adjustTargetBid(c: Campaign, targetId: string, multiplier: number): Campaign { const t = c.targets.find((x) => x.id === targetId); if (!t) return c; - return setTargetBid(c, targetId, t.bid * multiplier); + // A relative nudge (e.g. the "-10%" button), not an explicit "set to X" — + // floor at the platform minimum instead of throwing when decrementing an + // already-cheap bid. + return setTargetBid(c, targetId, Math.max(MIN_BID, t.bid * multiplier)); } export function pauseTarget(c: Campaign, targetId: string): Campaign { diff --git a/src/engine/ad-console/core/simulation.ts b/src/engine/ad-console/core/simulation.ts index c3dde86..7df376a 100644 --- a/src/engine/ad-console/core/simulation.ts +++ b/src/engine/ad-console/core/simulation.ts @@ -74,31 +74,28 @@ export function simulateDays(campaigns: Campaign[], days: number = 7): Campaign[ // Supports SP and SB campaigns (SD doesn't have search terms by design) const generatedST: SearchTerm[] = []; const shouldGenerateSearchTerms = c.type === 'SP' || c.type === 'SB'; - + // Tracks every term already in the campaign or generated this pass, so + // duplicate checks are O(1) instead of two linear scans per candidate + // (c.searchTerms only grows across simulation runs, so this mattered). + const seenTerms = new Set(c.searchTerms.map((st) => st.term)); + if (shouldGenerateSearchTerms) { for (let si = 0; si < enabledTargets.length; si++) { const tgt = enabledTargets[si]; if (tgt.type !== 'Keyword') continue; - + // Use the new generator with negative filtering during generation const generatedTerms = generateSearchTermsForTarget( tgt.value, tgt.match as 'Exact' | 'Phrase' | 'Broad', c.negatives.map(n => ({ value: n.value, type: n.type })) ); - + for (let gi = 0; gi < generatedTerms.length; gi++) { const gt = generatedTerms[gi]; - // Check if already exists in campaign search terms (avoid duplicates) - let exists = false; - for (let ei = 0; ei < c.searchTerms.length; ei++) { - if (c.searchTerms[ei].term === gt) { exists = true; break; } - } - for (let ei = 0; ei < generatedST.length; ei++) { - if (generatedST[ei].term === gt) { exists = true; break; } - } - if (exists) continue; - + if (seenTerms.has(gt)) continue; + seenTerms.add(gt); + const termShare = 0.15 + Math.random() * 0.1; const termClicks = Math.max(1, Math.round(tgt.clicks * termShare)); const termSpend = tgt.spend * termShare; diff --git a/src/engine/ad-console/core/slices/core.ts b/src/engine/ad-console/core/slices/core.ts index 93e388f..35dd732 100644 --- a/src/engine/ad-console/core/slices/core.ts +++ b/src/engine/ad-console/core/slices/core.ts @@ -36,7 +36,7 @@ export interface CoreSlice { } const coreState: AdConsoleState = { - version: '3.6', + version: '3.6.0', campaigns: defaultCampaigns(), filter: { type: 'All', status: 'All', portfolio: 'All', search: '' }, selectedCampaignId: null, @@ -67,7 +67,7 @@ export const createCoreSlice = (set: any, get: any, ..._rest: any[]): CoreSlice launchCampaign: () => set((s: any) => { const d = s.draft; if (!d.name.trim()) return s; - const id = 'C-' + d.type + '-' + Date.now().toString(36); + const id = generateId('C-' + d.type); const agId = 'AG-' + id; const portfolioName = d.portfolio || 'Training Portfolio'; const buildTargets = (raw: string, type: 'Keyword' | 'ASIN' | 'Category' | 'Audience - views remarketing', match?: any) => diff --git a/src/engine/ad-console/features/integrity/__tests__/engine.test.ts b/src/engine/ad-console/features/integrity/__tests__/engine.test.ts index 3b4955a..5d69d22 100644 --- a/src/engine/ad-console/features/integrity/__tests__/engine.test.ts +++ b/src/engine/ad-console/features/integrity/__tests__/engine.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { runIntegrityCheck } from '../engine'; import type { Campaign } from '../../../core/types'; @@ -114,4 +114,25 @@ describe('runIntegrityCheck', () => { it('fails fast when campaigns is not an array', () => { expect(() => runIntegrityCheck(null as unknown as Campaign[])).toThrow(); }); + + it('assigns unique issue ids even when generated within the same millisecond', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1700000000000); + try { + const report = runIntegrityCheck([ + baseCampaign({ + status: 'Archived', + targets: [ + { id: 'T1', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'a', match: 'Exact', bid: 0.75, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, + { id: 'T2', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'b', match: 'Exact', bid: 0.75, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, + { id: 'T3', campaignId: 'C1', adGroupId: 'AG1', type: 'Keyword', value: 'c', match: 'Exact', bid: 0.75, status: 'Enabled', impressions: 0, clicks: 0, spend: 0, sales: 0, orders: 0 }, + ], + }), + ]); + const ids = report.issues.map((i) => i.id); + expect(ids.length).toBeGreaterThan(1); + expect(new Set(ids).size).toBe(ids.length); + } finally { + nowSpy.mockRestore(); + } + }); }); diff --git a/src/engine/ad-console/features/integrity/engine.ts b/src/engine/ad-console/features/integrity/engine.ts index 5ebc3e6..f7f4b66 100644 --- a/src/engine/ad-console/features/integrity/engine.ts +++ b/src/engine/ad-console/features/integrity/engine.ts @@ -7,11 +7,10 @@ import type { Campaign } from '../../core/types'; import type { IntegrityIssue, IntegrityReport } from './types'; import { ValidationError } from '../../../../lib/validation'; +import { generateId } from '../../core/engine/id'; -let _counter = 0; function uid(): string { - _counter++; - return 'II-' + Date.now().toString(36) + '-' + _counter; + return generateId('II'); } export function runIntegrityCheck(campaigns: Campaign[]): IntegrityReport { diff --git a/src/engine/ad-console/features/profiles/__tests__/engine.test.ts b/src/engine/ad-console/features/profiles/__tests__/engine.test.ts index d6fb2b2..92ee038 100644 --- a/src/engine/ad-console/features/profiles/__tests__/engine.test.ts +++ b/src/engine/ad-console/features/profiles/__tests__/engine.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { createProfile, switchProfile, @@ -17,6 +17,17 @@ describe('createProfile', () => { it('falls back to a default name when blank', () => { expect(createProfile(' ').name).toBe('Trainee'); }); + + it('assigns unique ids even when created within the same millisecond', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1700000000000); + try { + const a = createProfile('Ana'); + const b = createProfile('Ben'); + expect(a.id).not.toBe(b.id); + } finally { + nowSpy.mockRestore(); + } + }); }); describe('profile collection ops', () => { diff --git a/src/engine/ad-console/features/profiles/engine.ts b/src/engine/ad-console/features/profiles/engine.ts index df054e5..c2cdbd1 100644 --- a/src/engine/ad-console/features/profiles/engine.ts +++ b/src/engine/ad-console/features/profiles/engine.ts @@ -3,16 +3,11 @@ */ import type { TraineeProfile } from './types'; import { assertNonEmpty } from '../../../../lib/validation'; - -let _counter = 0; -function uid(): string { - _counter++; - return 'P-' + Date.now().toString(36) + '-' + _counter; -} +import { generateId } from '../../core/engine/id'; export function createProfile(name: string): TraineeProfile { return { - id: uid(), + id: generateId('P'), name: name.trim().slice(0, 25) || 'Trainee', createdAt: new Date().toISOString(), lastActiveAt: new Date().toISOString(), diff --git a/src/engine/ad-console/features/reports/__tests__/engine.test.ts b/src/engine/ad-console/features/reports/__tests__/engine.test.ts index e39806e..5c1941d 100644 --- a/src/engine/ad-console/features/reports/__tests__/engine.test.ts +++ b/src/engine/ad-console/features/reports/__tests__/engine.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { createReportRequest, generateReport, reportToCsv } from '../engine'; import type { ReportType } from '../types'; @@ -14,6 +14,19 @@ describe('createReportRequest', () => { it('fails fast on unknown report type', () => { expect(() => createReportRequest('bogus' as ReportType)).toThrow(); }); + + it('assigns unique ids even when created within the same millisecond, including across createReportRequest and generateReport', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1700000000000); + try { + const reqA = createReportRequest('campaign'); + const reqB = createReportRequest('target'); + const report = generateReport('campaign'); + const ids = [reqA.id, reqB.id, report.id]; + expect(new Set(ids).size).toBe(ids.length); + } finally { + nowSpy.mockRestore(); + } + }); }); describe('generateReport', () => { diff --git a/src/engine/ad-console/features/reports/engine.ts b/src/engine/ad-console/features/reports/engine.ts index b0b9b38..a8c355c 100644 --- a/src/engine/ad-console/features/reports/engine.ts +++ b/src/engine/ad-console/features/reports/engine.ts @@ -2,20 +2,15 @@ * Reports — pure engine. */ import type { Report, ReportRequest, ReportType } from './types'; -import { assertNonEmpty, ValidationError } from '../../../../lib/validation'; - -let _counter = 0; -function uid(): string { - _counter++; - return 'R-' + Date.now().toString(36) + '-' + _counter; -} +import { ValidationError } from '../../../../lib/validation'; +import { generateId } from '../../core/engine/id'; const REPORT_TYPES: ReportType[] = ['campaign', 'adGroup', 'target', 'searchTerm', 'placement']; export function createReportRequest(type: ReportType): ReportRequest { if (!REPORT_TYPES.includes(type)) throw new ValidationError(`Unknown report type: ${type}`); return { - id: uid(), + id: generateId('R'), type, status: 'pending', requestedAt: new Date().toISOString(), @@ -49,7 +44,7 @@ export function generateReport(type: ReportType): Report { } return { - id: uid(), + id: generateId('R'), type, rows, generatedAt: now, diff --git a/src/engine/ad-console/features/trainer/__tests__/engine.test.ts b/src/engine/ad-console/features/trainer/__tests__/engine.test.ts index 2438e2b..a36091b 100644 --- a/src/engine/ad-console/features/trainer/__tests__/engine.test.ts +++ b/src/engine/ad-console/features/trainer/__tests__/engine.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { addNote, calculateCertScore, calculateGrade } from '../engine'; describe('addNote', () => { @@ -12,6 +12,17 @@ describe('addNote', () => { it('fails fast on empty note text', () => { expect(() => addNote(' ')).toThrow(); }); + + it('assigns unique ids even when created within the same millisecond', () => { + const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1700000000000); + try { + const a = addNote('first'); + const b = addNote('second'); + expect(a.id).not.toBe(b.id); + } finally { + nowSpy.mockRestore(); + } + }); }); describe('calculateCertScore', () => { diff --git a/src/engine/ad-console/features/trainer/engine.ts b/src/engine/ad-console/features/trainer/engine.ts index 34b950b..2117482 100644 --- a/src/engine/ad-console/features/trainer/engine.ts +++ b/src/engine/ad-console/features/trainer/engine.ts @@ -3,17 +3,12 @@ */ import type { TrainerNote, TrainerState } from './types'; import { assertNonEmpty } from '../../../../lib/validation'; - -let _counter = 0; -function uid(): string { - _counter++; - return 'TN-' + Date.now().toString(36) + '-' + _counter; -} +import { generateId } from '../../core/engine/id'; export function addNote(text: string): TrainerNote { assertNonEmpty('note text', text); return { - id: uid(), + id: generateId('TN'), timestamp: new Date().toISOString(), text: text.trim(), }; diff --git a/src/lib/validation.ts b/src/lib/validation.ts index 54eaa17..6220f5d 100644 --- a/src/lib/validation.ts +++ b/src/lib/validation.ts @@ -18,6 +18,23 @@ export function assertFiniteNonNegative(name: string, value: number): void { } } +/** Amazon Ads' real minimum bid — used by functions that set an explicit bid on existing entities. */ +export const MIN_BID = 0.02; + +/** + * Like assertFiniteNonNegative, but also enforces the platform's real bid + * floor. Use this for "set the bid to X" actions on an existing entity, + * where silently substituting a different value than what was explicitly + * requested would be misleading. Creation/normalization paths that fill in + * defaults for otherwise-incomplete data should keep clamping instead. + */ +export function assertValidBid(name: string, value: number): void { + assertFiniteNonNegative(name, value); + if (value < MIN_BID) { + throw new ValidationError(`${name} must be at least $${MIN_BID.toFixed(2)}, got ${value}`); + } +} + export function assertNonEmpty(name: string, value: string): void { if (typeof value !== 'string' || value.trim().length === 0) { throw new ValidationError(`${name} must be a non-empty string`);