From c3fc17fd803761cfc9c89a68fd6b6c9210891555 Mon Sep 17 00:00:00 2001 From: Anil Murty Date: Fri, 25 Sep 2026 11:12:50 -0700 Subject: [PATCH 1/3] Lens: the hero CTA goes to the biggest opportunity, not a retired route "Review N fixes" pointed at #/review. That page was retired when its apply lifecycle moved onto the Optimize detail pages, and primaryKeyFor aliases the hash to the Dashboard so old bookmarks do not 404. The button therefore re-rendered the page the reader was already on: the address bar changed and nothing else did. Critical Rule 24(c) is about exactly this, checking that a destination resolves rather than that a link exists. The CTA now builds its href from the top opportunity, which is the page that can apply the fix and the one the sub-line already names. The test pins the inverse of the defect rather than deleting anything: the href must be computed, and #/review must not appear as a link target. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test_lens_dashboard_states.py | 21 +++++++++++++++++++++ tokenjam/ui/index.html | 10 +++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_lens_dashboard_states.py b/tests/unit/test_lens_dashboard_states.py index 6aff281a..2ae4e6e1 100644 --- a/tests/unit/test_lens_dashboard_states.py +++ b/tests/unit/test_lens_dashboard_states.py @@ -671,3 +671,24 @@ def test_no_dollar_surface_renders_a_suppressed_figure_with_no_explanation(): ): result = _run_dedup_js(expr) assert result not in (None, ""), "%s produced no explanation: %r" % (expr, result) + + +def test_the_hero_cta_goes_to_a_route_that_resolves_to_its_own_surface(html: str): + """The "Review N fixes" CTA used to point at `#/review`, a page retired when + the apply lifecycle moved onto the Optimize detail pages. `primaryKeyFor` + aliases `#/review` to the Dashboard so old bookmarks do not 404, which meant + the button re-rendered the page the reader was already on. Critical Rule + 24(c): check the destination RESOLVES, not merely that a link exists. + + The inverse of the defect is pinned, per Critical Rule 23: the hero's href + must be built from the top opportunity, and `#/review` must not appear as a + destination anywhere in the file. + """ + assert 'const ctaHref = opps.length ? optimizeFindingHref(opps[0].name)' in html, ( + "the hero CTA must target the biggest opportunity's detail page" + ) + assert 'href=${ctaHref}' in html, "the hero CTA must render the computed href" + assert 'href="#/review"' not in html, ( + "#/review aliases to the Dashboard; a link there is a no-op for a reader " + "already on the Dashboard" + ) diff --git a/tokenjam/ui/index.html b/tokenjam/ui/index.html index 19815b16..3fc856da 100644 --- a/tokenjam/ui/index.html +++ b/tokenjam/ui/index.html @@ -7389,6 +7389,14 @@

${friendlySpanName(sel.name)}

} const opps = heroOpps(fig, framing); const topName = opps.length ? opps[0].title : ''; + // The CTA goes to the BIGGEST opportunity's detail page, where the apply + // lifecycle actually lives. It used to point at `#/review`, a page retired + // when that lifecycle moved onto the Optimize details: `primaryKeyFor` + // aliases `#/review` to the Dashboard so old bookmarks do not 404, so the + // button re-rendered the page the reader was already on (Critical Rule 24c: + // the destination did not resolve). With no priced opportunity the hero + // renders its empty state above and never reaches here. + const ctaHref = opps.length ? optimizeFindingHref(opps[0].name) : '#/optimize'; // The share phrase ("of $Y spent · Z%") only makes sense in dollar mode — // in local/token-only mode there is no marginal spend to divide by, and // the hero figure itself already fell back to a token count via @@ -7418,7 +7426,7 @@

${friendlySpanName(sel.name)}

${heroSentence}
- ${ctaLabel} → + ${ctaLabel} → ${topName ? html`
starts with the biggest one: ${topName}
` : null}
From f428ddb23076d4435d48a0997b34408c6f8b0f33 Mon Sep 17 00:00:00 2001 From: Anil Murty Date: Sat, 26 Sep 2026 09:52:24 -0700 Subject: [PATCH 2/3] Address review: resolve the CTA to a surface that renders the analyzer Greptile, both valid. P1: the fix reintroduced the defect it removed, one analyzer along. relearn is not in DETAIL_ANALYZER_NAMES, so when a priced relearn cluster is the biggest contributor the CTA opened #/optimize/relearn, which renders "no relearn finding in the latest scan" under a button promising a fix. analyzerSurfaceHref now resolves each analyzer to a surface that actually renders it: its detail card, the Rules view for relearn, the Summarize view for summarize, and the ranked Optimize landing for anything with no surface of its own. P2: the test pattern-matched the source that builds the href, so it would have stayed green if the helper or the router later sent readers back to the Dashboard. It now lifts analyzerSurfaceHref, optimizeFindingHref and primaryKeyFor out of the page and RESOLVES each analyzer through the real router, asserting the primary view key is never 'dashboard'. Parametrised over all eleven detail analyzers plus the three without a detail card. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test_lens_dashboard_states.py | 93 +++++++++++++++++++----- tokenjam/ui/index.html | 23 +++++- 2 files changed, 95 insertions(+), 21 deletions(-) diff --git a/tests/unit/test_lens_dashboard_states.py b/tests/unit/test_lens_dashboard_states.py index 2ae4e6e1..ddf12ed2 100644 --- a/tests/unit/test_lens_dashboard_states.py +++ b/tests/unit/test_lens_dashboard_states.py @@ -673,22 +673,79 @@ def test_no_dollar_surface_renders_a_suppressed_figure_with_no_explanation(): assert result not in (None, ""), "%s produced no explanation: %r" % (expr, result) -def test_the_hero_cta_goes_to_a_route_that_resolves_to_its_own_surface(html: str): - """The "Review N fixes" CTA used to point at `#/review`, a page retired when - the apply lifecycle moved onto the Optimize detail pages. `primaryKeyFor` - aliases `#/review` to the Dashboard so old bookmarks do not 404, which meant - the button re-rendered the page the reader was already on. Critical Rule - 24(c): check the destination RESOLVES, not merely that a link exists. - - The inverse of the defect is pinned, per Critical Rule 23: the hero's href - must be built from the top opportunity, and `#/review` must not appear as a - destination anywhere in the file. - """ - assert 'const ctaHref = opps.length ? optimizeFindingHref(opps[0].name)' in html, ( - "the hero CTA must target the biggest opportunity's detail page" - ) - assert 'href=${ctaHref}' in html, "the hero CTA must render the computed href" - assert 'href="#/review"' not in html, ( - "#/review aliases to the Dashboard; a link there is a no-op for a reader " - "already on the Dashboard" +def _cta_destination_source() -> str: + """`analyzerSurfaceHref` + `optimizeFindingHref` + `primaryKeyFor` + the + constants they read, lifted verbatim from the served page, so a test can + RESOLVE a CTA rather than pattern-match the source that builds it.""" + src = _UI.read_text(encoding="utf-8") + pieces = [] + for start_marker, end_marker in ( + ("const DEFAULT_SINCE", "\n"), + ("const DETAIL_ANALYZER_NAMES = new Set([", "]);"), + ("function analyzerSurfaceHref", "\n}"), + ("function optimizeFindingHref", "\n}"), + ("const SESSIONS_SDK_TAB_VIEWS", "\n"), + ("const UNSURFACED_VIEWS", "\n"), + ("function primaryKeyFor", "\n}"), + ): + i = src.index(start_marker) + j = src.index(end_marker, i) + len(end_marker) + pieces.append(src[i:j]) + return "\n".join(pieces) + + +def _resolve(analyzer: str) -> dict: + """The href the hero CTA builds for `analyzer`, and the primary view key + the router resolves it to. A CTA whose key is 'dashboard' is the defect.""" + script = _cta_destination_source() + f""" +const href = analyzerSurfaceHref({analyzer!r}); +const hash = href.replace(/^#\//, ''); +const [path, qs] = hash.split('?'); +const [view, param] = path.split('/'); +console.log(JSON.stringify({{ href, key: primaryKeyFor({{ view, param }}) }})); +""" + proc = subprocess.run( + ["node", "--input-type=module", "-e", script], + capture_output=True, text=True, check=True, ) + return json.loads(proc.stdout.strip()) + + +@pytest.mark.parametrize("analyzer", [ + "downsize", "cache", "script", "trim", "reuse", "subagent", "verbosity", + "deadweight", "placement", "resend", "cache-recommend", +]) +def test_every_detail_analyzer_cta_resolves_to_its_own_optimize_page(analyzer: str): + """Critical Rule 24(c): check the destination RESOLVES, not that a link + exists. The defect this replaced put `#/review` here, which `primaryKeyFor` + aliases to 'dashboard', so the button re-rendered the page the reader was + already on.""" + got = _resolve(analyzer) + assert got["key"] == "optimize", f"{analyzer} CTA resolved to {got['key']} via {got['href']}" + assert got["href"] == f"#/optimize/{analyzer}" + + +@pytest.mark.parametrize("analyzer,expected_key", [ + ("relearn", "rules"), # its surface is the Rules view, labelled Relearn + ("summarize", "summarize"), # its own view, not an OptimizeFinding card + ("shipped", "optimize"), # no detail card: the ranked landing, never a dead route +]) +def test_analyzers_without_a_detail_card_reach_a_surface_that_renders_them( + analyzer: str, expected_key: str, +): + """An analyzer with no OptimizeFinding card must not get `#/optimize/`: + that route renders "no finding in the latest scan" under a button promising + a fix. Relearn is the live case, it can be the biggest contributor and has + no detail card.""" + got = _resolve(analyzer) + assert got["key"] == expected_key, f"{analyzer} resolved to {got['key']} via {got['href']}" + assert got["key"] != "dashboard" + + +def test_the_hero_cta_is_computed_and_review_is_not_a_link_target(html: str): + """The inverse of the original defect, pinned per Critical Rule 23 rather + than deleted: the href must be built from the top opportunity, and no link + anywhere may target the retired `#/review` route.""" + assert "const ctaHref = opps.length ? analyzerSurfaceHref(opps[0].name)" in html + assert "href=${ctaHref}" in html + assert 'href="#/review"' not in html diff --git a/tokenjam/ui/index.html b/tokenjam/ui/index.html index 3fc856da..253dc56c 100644 --- a/tokenjam/ui/index.html +++ b/tokenjam/ui/index.html @@ -7389,14 +7389,17 @@

${friendlySpanName(sel.name)}

} const opps = heroOpps(fig, framing); const topName = opps.length ? opps[0].title : ''; - // The CTA goes to the BIGGEST opportunity's detail page, where the apply - // lifecycle actually lives. It used to point at `#/review`, a page retired + // The CTA goes to the surface that renders the BIGGEST opportunity, where + // the apply lifecycle actually lives. `analyzerSurfaceHref` resolves it: + // relearn and summarize have their own views, and an analyzer with no + // surface falls back to the ranked Optimize landing rather than a detail + // route that would render "no finding in the latest scan". It used to point at `#/review`, a page retired // when that lifecycle moved onto the Optimize details: `primaryKeyFor` // aliases `#/review` to the Dashboard so old bookmarks do not 404, so the // button re-rendered the page the reader was already on (Critical Rule 24c: // the destination did not resolve). With no priced opportunity the hero // renders its empty state above and never reaches here. - const ctaHref = opps.length ? optimizeFindingHref(opps[0].name) : '#/optimize'; + const ctaHref = opps.length ? analyzerSurfaceHref(opps[0].name) : '#/optimize'; // The share phrase ("of $Y spent · Z%") only makes sense in dollar mode — // in local/token-only mode there is no marginal spend to divide by, and // the hero figure itself already fell back to a token count via @@ -7460,6 +7463,20 @@

${friendlySpanName(sel.name)}

// filtered `order` array can't drift apart). const OPT_DETAIL_ORDER = ['downsize', 'resend', 'cache', 'cache-recommend', 'script', 'trim', 'reuse', 'subagent', 'verbosity', 'deadweight', 'placement']; +// Where an analyzer's fix actually lives. Most render an OptimizeFinding +// detail card (DETAIL_ANALYZER_NAMES); `summarize` has its own view and +// `relearn` is the Rules view (the sidebar labels that route "Relearn"). An +// analyzer with no surface of its own resolves to the Optimize landing, which +// ranks every finding — never to a route that renders "no finding in the +// latest scan" under a button promising a fix (Critical Rule 24c). Used by the +// Dashboard hero CTA, whose target is whichever analyzer is biggest. +function analyzerSurfaceHref(name, since) { + if (name === 'summarize') return '#/optimize/summarize'; + if (name === 'relearn') return '#/optimize/rules'; + if (DETAIL_ANALYZER_NAMES.has(name)) return optimizeFindingHref(name, since); + return '#/optimize'; +} + // The mech badge (design's apply/snippet/read pill) is a static // classification of an analyzer's fix MECHANISM, not a per-instance // computation — same status as ANALYZER_META's hint text, which is why it From 515f69d23db6eb98b1ca1175d0e5ceb481619b9b Mon Sep 17 00:00:00 2001 From: Anil Murty Date: Sat, 26 Sep 2026 10:41:25 -0700 Subject: [PATCH 3/3] Skip the CTA resolution tests when node is absent Greptile, valid: the two new parametrized tests shell out to node through _resolve, so without it they failed with FileNotFoundError instead of skipping. The file already carries the _node marker the other JS tests use; these now use it too. Verified by running the file with node off PATH: 28 passed, 36 skipped, no errors. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test_lens_dashboard_states.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_lens_dashboard_states.py b/tests/unit/test_lens_dashboard_states.py index ddf12ed2..52df7459 100644 --- a/tests/unit/test_lens_dashboard_states.py +++ b/tests/unit/test_lens_dashboard_states.py @@ -711,6 +711,7 @@ def _resolve(analyzer: str) -> dict: return json.loads(proc.stdout.strip()) +@_node @pytest.mark.parametrize("analyzer", [ "downsize", "cache", "script", "trim", "reuse", "subagent", "verbosity", "deadweight", "placement", "resend", "cache-recommend", @@ -725,6 +726,7 @@ def test_every_detail_analyzer_cta_resolves_to_its_own_optimize_page(analyzer: s assert got["href"] == f"#/optimize/{analyzer}" +@_node @pytest.mark.parametrize("analyzer,expected_key", [ ("relearn", "rules"), # its surface is the Rules view, labelled Relearn ("summarize", "summarize"), # its own view, not an OptimizeFinding card