Skip to content

fix(gh-389): label the city HUD's two object counts - #405

Open
db-tycoon-stephen wants to merge 1 commit into
fix/gh-388-view-rowsfrom
fix/gh-389-labelled-counts
Open

db-tycoon-stephen wants to merge 1 commit into
fix/gh-388-view-rowsfrom
fix/gh-389-labelled-counts

Conversation

@db-tycoon-stephen

@db-tycoon-stephen db-tycoon-stephen commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

What this does

The footer said "90 objects" and the run replay header said "109 nodes off this map", with nothing to say they count different things. The footer now reads "90 tables and views" and the run header "109 dbt nodes with no building (tests, seeds, models outside this catalog)". Also rebuilds web_dist.

Mental model

Each number names its unit: catalog relations versus one run's dbt nodes that have no building.

Linked issue

Closes #389

Type of change

  • Bug fix

Checklist

  • pre-commit run --all-files passes
  • Tests pass locally, and the offline csv-import pipeline still runs end to end (this is the CI gate — if it breaks, the release is blocked)
  • Added or updated tests where it made sense
  • Updated CHANGELOG.md if this is user-facing
  • Updated docs / README if behavior or commands changed

Notes for reviewers

refresh.spec.ts's "99 objects" assertion was updated to match. The Stats modal heading and the plant inspector still say "objects"; they aren't next to the run count.

Depends on

The shipped web_dist bundle is rebuilt here, so whichever city PR merges later needs scripts/sync_web_bundle.py rerun.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

PR size check

mode: warn | file limit: 8

Files changed: 7 counted / 8 allowed

  • CHANGELOG.md
  • src/tycoon_city/web_dist/assets/index-BkALontW.js
  • src/tycoon_city/web_dist/index.html
  • web/e2e/refresh.spec.ts
  • web/e2e/run_replay.spec.ts
  • web/src/boot/hud.ts
  • web/src/ui/run_panel.ts

Result: ✅ within limits.

@github-actions

Copy link
Copy Markdown

PR title check

mode: warn

fix(gh-389): label the city HUD's two object counts

Result: ✅ title matches the convention.

@db-tycoon-stephen
db-tycoon-stephen changed the base branch from v0.2.3 to fix/gh-388-view-rows October 13, 2026 10:50
@db-tycoon-stephen db-tycoon-stephen moved this to Todo in Tycoon CLI Oct 13, 2026
@db-tycoon-stephen db-tycoon-stephen added this to the v0.2.3 milestone Oct 13, 2026
@db-tycoon-stephen db-tycoon-stephen self-assigned this Oct 13, 2026
@db-tycoon-stephen db-tycoon-stephen modified the milestones: v0.2.3, v0.2.4 Oct 21, 2026
The footer showed "90 objects" and the run replay header showed
"109 nodes off this map" on the same screen (gh-389). They count
different things: tables and views in the catalog, and dbt nodes in one
run that have no building (tests, seeds, models the catalog does not
have). Read bare, they look like two answers to one question. Each now
says what it counts.

Rebuilds the shipped web bundle so CI's freshness check stays green.

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

@db-tycoon-stephen db-tycoon-stephen left a comment •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Independent review at f6ab71b. This replaces an earlier automated comment that was not based on the diff.

Verdict: approve, with two follow-ups.

  • The footer now counts tables and views only, which matches how object_count is built.
  • The run header counts dbt nodes with no building (tests, seeds, models outside this catalog), which matches docs/city/run-json-v1.md.
  • Follow-up: web/src/ui/tour_stops.ts:107 and docs/city/hud-design.md:58 still say "objects".
  • Follow-up: on catalogs over MAX_OBJECTS (500), the footer shows the capped count; the cap note in the notes popover is the only hint.

@JesuFemi-O JesuFemi-O left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: approve with fixes. The footer label is accurate and the bundle and tests hold up; the run header label says "no building" for seeds and snapshots that do have one, which should be reworded before it ships.

What it does

Footer N objects becomes N tables and views (web/src/boot/hud.ts:85). The run replay header N nodes off this map becomes N dbt nodes with no building (tests, seeds, models outside this catalog) (web/src/ui/run_panel.ts:206). Adds one e2e test, updates one assertion in refresh.spec.ts, adds a CHANGELOG entry, rebuilds web_dist.

Checked (at f6ab71b, with #401 to #404 below it)

  • Footer count: object_count is len(ctx.objects) (catalog/models.py:100), built only from DuckDB tables and views (catalog/loader.py:139-143), after the 500 object cap. Sources, tests, columns, roads and power lines are not in it. On a demo-tycoon project plus one added view and one added table the document reports 12 objects (11 table, 1 view) and the footer reads 12 tables and views. The label is true.
  • Run header count: unmapped_count is the run nodes whose unique_id is not a value of ctx.dbt_nodes_by_key (export/run_json.py:105-119). dbt_nodes_by_key holds models only (catalog/dbt_manifest.py, nodes_by_key). CONFIRMED, see Should fix 1.
  • Bundle: scripts/sync_web_bundle.py produces no diff on the branch. The minified diff against the base bundle contains only the two string changes (footer template, run header template) and the renamed asset reference in index.html. No hidden source change.
  • Screenshots, shipped bundle served over the same exported project, before vs after. At 1280x800 the footer went from 12 objects to 12 tables and views. The run panel header went from 2 lines to 3 and the panel grew 13px, with no overflow or clipping (panel scroll size equals client size). At 420x800 the run panel fits and the header wraps to 3 lines. At 420 the footer #status is not visible in either build (client width 0, ellipsis, the lens/exported/notes controls take the row), so the new label is not readable there before or after.
  • Footer clipping by width (measured #status client vs scroll width): after the change the text needs 354px instead of 289px. At 900px viewport and above it fits. At 760px it shows 246 of 354px (before: 246 of 289), at 640px 126 of 354, at 520px 6 of 354. The new label is cut mid-phrase earlier than the old one, and the rows figure is the first thing lost. This sits on top of #403's clamping and is not a regression of it.
  • Tests: npx tsc --noEmit clean. npx playwright test 137 passed (after npm run demo-data; without web/public every test times out, which is an environment precondition, not a PR issue). uv run pytest -q 1575 passed, 10 skipped, 1 deselected. ruff check and ruff format --check clean.
  • Mutation check: reverting the footer string to objects fails the new test; reverting the run header string fails it too. The refresh.spec.ts edit only swaps the expected string, so it is not weakened.
  • Repo rules: 7 files, under the cap of 8. CHANGELOG entry present with the link reference. No em dashes in added lines. The template box for docs is unticked and no docs changed.

Should fix

  1. web/src/ui/run_panel.ts:206: the label says "no building (tests, seeds, models outside this catalog)", but a seed or snapshot whose table is in the catalog does have a building and is still counted. CONFIRMED: on a project whose warehouse contains raw.country_codes (seeded by seed.fx_dbt.country_codes) with a run containing that seed, a snapshot, a test and a model not in the catalog, the run document gives unmapped_count 4 with kinds model, seed, snapshot, test, while the catalog has a building for the seed's table. The cause is that only models are joined to buildings (nodes_by_key is models only), so seeds and snapshots are always unmapped. Snapshots are also missing from the parenthetical. Reword so it states what is true, for example "dbt nodes not matched to a building (tests, seeds, snapshots, models outside this catalog)". The same imprecision is in docs/city/run-json-v1.md:81; worth correcting there in the same PR or a follow-up.

Nits

  • web/src/ui/tour_stops.ts:107 (tour "plant" stop) and web/src/ui/stats.ts:67 (Stats heading) still print the same object_count as N objects, so the app now names one number two ways. The PR body acknowledges the Stats heading and inspector; the tour stop is the one a first-time user reads next to the footer. docs/city/hud-design.md:58 also still says objects. PLAUSIBLE that this confuses anyone; not reproduced as user confusion.
  • On catalogs over 500 objects the footer shows the capped count as N tables and views, which reads as the whole database. The cap is only named in the notes popover. Not changed by this PR; reproduced from the code path only, so PLAUSIBLE.
  • The new test pins the strings against a fixture whose unmapped nodes are a test and a seed, so it would not catch the wording problem above. A fixture with a seed that has a table would.

Not checked

  • A catalog larger than 500 objects, and a real dbt project with snapshots and sources in a run (the extra nodes above were added by hand to a generated demo project).
  • Safari and Firefox rendering; screenshots were Chrome with software GL.
  • mkdocs build --strict (no docs changed), pre-commit, CI itself.
  • Upgrade from v0.2.3: the change is display text only and reads no new field, but no v0.2.3 project was run against this branch.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants