Add Gate 2 thread visualizer (descent + break demo) - #7
Merged
Conversation
Walker projects live PRD/@covers/test data into JSON matching Gate 2 totals; descent UI shows one requirement unbroken, with a seeded break on AC-USER-04. Co-authored-by: Rik Dryfoos <rdryfoos@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds a read-only HomesFlow Thread Visualizer. A Python walker builds Gate 2 traceability projections from live PRD, source, and test data. The browser renders intact and seeded broken proof states. Documentation and a refresh gate describe regeneration. ChangesThread Visualizer
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant RefreshScript as thread-visualizer-refresh.sh
participant Walker as thread-walker.py
participant Data as Gate 2, PRD, Swift, and test data
participant Browser
Developer->>RefreshScript: run refresh command
RefreshScript->>Walker: generate projections
Walker->>Data: read and resolve traceability data
Walker-->>RefreshScript: write thread.json and thread-broken.json
Browser->>Browser: load visualizer page
Browser->>Data: fetch generated projection
Browser->>Browser: render selected requirement and proof state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
docs/thread-visualizer/SPEC.md (1)
32-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a language to the fenced code block.
markdownlint reports MD040 on this block. Add
textafter the opening fence.📝 Proposed fix
-``` +```text objective/requirement (PRD registry ID + AC text)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/thread-visualizer/SPEC.md` around lines 32 - 37, Update the fenced code block in the objective/requirement traceability section by adding the text language identifier to its opening fence, while preserving the block’s contents unchanged.Source: Linters/SAST tools
docs/thread-visualizer/visualizer.css (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStylelint flags the
Georgiakeyword case.Stylelint reports
value-keyword-caseon line 15. Either lowercase the value or quote it so the rule treats it as a family name string.🎨 Proposed fix
- --display: "Fraunces", Georgia, serif; + --display: "Fraunces", "Georgia", serif;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/thread-visualizer/visualizer.css` at line 15, Update the --display font-family value so the Georgia family name satisfies the value-keyword-case Stylelint rule, either by using lowercase text or quoting Georgia while preserving the existing font fallback order.Source: Linters/SAST tools
docs/thread-visualizer/visualizer.js (1)
56-77: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winEscape every interpolated value in the HTML templates.
escapeHtmlis applied to the requirement text only. These values go intoinnerHTMLunescaped:thread.id(lines 98, 110),thread.requirement.source(line 100),doneTasks(line 103),m.path(line 61),t.nameandt.path(lines 73-74), andt.idin the<option value>(line 47).The current data comes from the local walker, so the risk is contained today. The templates are still fragile if a path or PRD field ever carries
<,&, or". Route all interpolated values throughescapeHtml.🛡️ Proposed fix (representative)
- .map((m) => `<li><code>${m.path}</code></li>`) + .map((m) => `<li><code>${escapeHtml(m.path)}</code></li>`)- const where = t.path ? ` <span class="muted">in <code>${t.path}</code></span>` : ""; - return `<li><code>${t.name}</code>${where}</li>`; + const where = t.path ? ` <span class="muted">in <code>${escapeHtml(t.path)}</code></span>` : ""; + return `<li><code>${escapeHtml(t.name)}</code>${where}</li>`;- <p>Source: <code>${thread.requirement.source}</code></p> + <p>Source: <code>${escapeHtml(thread.requirement.source)}</code></p>Also applies to: 95-127, 138-149
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/thread-visualizer/visualizer.js` around lines 56 - 77, Update all HTML template interpolations in the visualizer, including moduleList, proofList, option values, and thread/task rendering, to pass every dynamic value such as thread.id, thread.requirement.source, doneTasks, m.path, t.name, t.path, and t.id through the existing escapeHtml helper before inserting them into innerHTML. Keep the surrounding markup and rendering behavior unchanged.Source: Linters/SAST tools
scripts/thread-walker.py (1)
116-166: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winScan the source tree once instead of once per AC.
files_coveringrunsrglob("*.swift")and reads every Swift file for each AC.files_for_testsdoes the same over the test directories.build_threadscalls both for all 50 ACs, so the walker reads the whole tree about 100 times.Build one map of ID → files and one map of test name → file before the loop, then look up per AC. Runtime drops from O(ACs × files) to O(files).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/thread-walker.py` around lines 116 - 166, Refactor files_covering and files_for_tests so Swift source and test trees are scanned once before build_threads processes ACs, constructing reusable maps from coverage IDs to implementation files and test names to test files. Update build_threads to build or receive these indexes and perform per-AC lookups instead of invoking full-tree scans, while preserving result formats, deduplication, and missing-test behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/thread-visualizer/index.html`:
- Around line 31-34: Update the toggle buttons btnIntact and btnBreak to expose
their selected state with aria-pressed, and synchronize those attributes in
setMode using the current mode so only the active button reports true.
In `@scripts/thread-walker.py`:
- Around line 221-251: Update scripts/thread-walker.py in apply_demo_break to
move totals recomputation outside the thread loop, recompute acsTrackedDebt, and
derive gate2Passed from the recomputed acsGap. Regenerate
docs/thread-visualizer/data/thread-broken.json using bash
scripts/thread-visualizer-refresh.sh so gate2Passed is false. In
docs/thread-visualizer/visualizer.js, derive gateOk solely from t.gate2Passed
and remove the state.mode === "intact" condition.
In `@specs/001-mvp/craft-conventions.md`:
- Line 94: Add CI coverage for the Thread visualizer craft gate by updating the
craft-gate workflow to run scripts/thread-visualizer-refresh.sh and regenerate
docs/thread-visualizer/data/ during validation, or remove the Thread visualizer
entry from the craft conventions if it is not required. Ensure the selected
approach prevents stale generated data from merging silently.
---
Nitpick comments:
In `@docs/thread-visualizer/SPEC.md`:
- Around line 32-37: Update the fenced code block in the objective/requirement
traceability section by adding the text language identifier to its opening
fence, while preserving the block’s contents unchanged.
In `@docs/thread-visualizer/visualizer.css`:
- Line 15: Update the --display font-family value so the Georgia family name
satisfies the value-keyword-case Stylelint rule, either by using lowercase text
or quoting Georgia while preserving the existing font fallback order.
In `@docs/thread-visualizer/visualizer.js`:
- Around line 56-77: Update all HTML template interpolations in the visualizer,
including moduleList, proofList, option values, and thread/task rendering, to
pass every dynamic value such as thread.id, thread.requirement.source,
doneTasks, m.path, t.name, t.path, and t.id through the existing escapeHtml
helper before inserting them into innerHTML. Keep the surrounding markup and
rendering behavior unchanged.
In `@scripts/thread-walker.py`:
- Around line 116-166: Refactor files_covering and files_for_tests so Swift
source and test trees are scanned once before build_threads processes ACs,
constructing reusable maps from coverage IDs to implementation files and test
names to test files. Update build_threads to build or receive these indexes and
perform per-AC lookups instead of invoking full-tree scans, while preserving
result formats, deduplication, and missing-test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89cfd463-1b5f-494e-8f98-ac3200bdca73
📒 Files selected for processing (11)
README.mddocs/thread-visualizer/README.mddocs/thread-visualizer/SPEC.mddocs/thread-visualizer/data/thread-broken.jsondocs/thread-visualizer/data/thread.jsondocs/thread-visualizer/index.htmldocs/thread-visualizer/visualizer.cssdocs/thread-visualizer/visualizer.jsscripts/thread-visualizer-refresh.shscripts/thread-walker.pyspecs/001-mvp/craft-conventions.md
Comment on lines
+31
to
+34
| <div class="break-toggle" role="group" aria-label="Thread state"> | ||
| <button type="button" id="btnIntact" class="chip is-active">Intact</button> | ||
| <button type="button" id="btnBreak" class="chip chip-danger">Break the proof</button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Expose the active toggle state to assistive technology.
btnIntact and btnBreak form a toggle pair. The active state is conveyed only by the is-active class and colour. A screen reader user cannot tell which state is selected. Add aria-pressed and keep it in sync in setMode.
♿ Proposed fix
- <button type="button" id="btnIntact" class="chip is-active">Intact</button>
- <button type="button" id="btnBreak" class="chip chip-danger">Break the proof</button>
+ <button type="button" id="btnIntact" class="chip is-active" aria-pressed="true">Intact</button>
+ <button type="button" id="btnBreak" class="chip chip-danger" aria-pressed="false">Break the proof</button>In docs/thread-visualizer/visualizer.js, inside setMode:
$("btnIntact").setAttribute("aria-pressed", String(mode === "intact"));
$("btnBreak").setAttribute("aria-pressed", String(mode === "broken"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/thread-visualizer/index.html` around lines 31 - 34, Update the toggle
buttons btnIntact and btnBreak to expose their selected state with aria-pressed,
and synchronize those attributes in setMode using the current mode so only the
active button reports true.
Comment on lines
+221
to
+251
| for thread in out["threads"]: | ||
| if thread["id"] != break_id: | ||
| continue | ||
| thread["proof"]["tests"] = [] | ||
| covered = thread["implementation"]["covered"] | ||
| pending = thread["requirement"]["pendingTasks"] | ||
| if covered and pending: | ||
| thread["gateStatus"] = "implemented-test-pending" | ||
| thread["status"] = VIS_TRACKED | ||
| elif covered and not pending: | ||
| thread["gateStatus"] = "gap" | ||
| thread["status"] = VIS_GAP | ||
| elif pending: | ||
| thread["gateStatus"] = "planned" | ||
| thread["status"] = VIS_TRACKED | ||
| else: | ||
| thread["gateStatus"] = "gap" | ||
| thread["status"] = VIS_GAP | ||
| out["totals"]["acsWithTests"] = sum( | ||
| 1 for t in out["threads"] if t["proof"]["tests"] | ||
| ) | ||
| out["totals"]["acsVerified"] = sum( | ||
| 1 for t in out["threads"] if t["status"] == VIS_VERIFIED | ||
| ) | ||
| out["totals"]["acsGap"] = sum( | ||
| 1 for t in out["threads"] if t["status"] == VIS_GAP | ||
| ) | ||
| break | ||
| else: | ||
| raise SystemExit(f"Demo break ID not found in threads: {break_id}") | ||
| return out |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Stale gate2Passed in the broken projection. apply_demo_break recomputes acsWithTests, acsVerified, and acsGap, but it copies gate2Passed and acsTrackedDebt from the intact run. Gate 2 fails the build on a silent gap, so the broken projection asserts a passing gate next to a GAP, and the UI has to work around it.
scripts/thread-walker.py#L221-L251: recomputeacsTrackedDebtand setgate2Passedfrom the recomputedacsGap. Move the totals recomputation out of the loop.docs/thread-visualizer/data/thread-broken.json#L9-L18: regenerate withbash scripts/thread-visualizer-refresh.shsogate2Passedbecomesfalse.docs/thread-visualizer/visualizer.js#L26-L35: after the data is correct, derivegateOkfromt.gate2Passedalone and drop thestate.mode === "intact"guard.
📍 Affects 3 files
scripts/thread-walker.py#L221-L251(this comment)docs/thread-visualizer/data/thread-broken.json#L9-L18docs/thread-visualizer/visualizer.js#L26-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/thread-walker.py` around lines 221 - 251, Update
scripts/thread-walker.py in apply_demo_break to move totals recomputation
outside the thread loop, recompute acsTrackedDebt, and derive gate2Passed from
the recomputed acsGap. Regenerate docs/thread-visualizer/data/thread-broken.json
using bash scripts/thread-visualizer-refresh.sh so gate2Passed is false. In
docs/thread-visualizer/visualizer.js, derive gateOk solely from t.gate2Passed
and remove the state.mode === "intact" condition.
| |------|---------|--------| | ||
| | **Gate 0** | `xcodebuild build` + `xcodebuild test` (`HomesFlowTests`) | macOS | | ||
| | **Gate 2** | `bash scripts/check-traceability.sh` | Linux or macOS | | ||
| | **Thread visualizer** | `bash scripts/thread-visualizer-refresh.sh` → `docs/thread-visualizer/` | Linux or macOS | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether any CI workflow invokes the thread visualizer refresh or the walker.
set -euo pipefail
fd -t f -e yml -e yaml . .github 2>/dev/null | while IFS= read -r f; do
rg -n -C3 'thread-visualizer|thread-walker' "$f" || true
done
# Also show the jobs declared in CI for context.
fd -t f 'ci.ya?ml' .github | xargs -r rg -n '^\s{0,4}[a-zA-Z0-9_-]+:\s*$' -m 60Repository: rdryfoos/HomesFlow
Length of output: 342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflows =="
git ls-files .github | sed -n '1,120p'
echo
echo "== thread visualizer references in workflows/docs/scripts =="
rg -n -C3 'thread-visualizer|thread-walker|craft-gate|craft-conventions|thread-visualizer-refresh' .github specs scripts docs.json 2>/dev/null || true
echo
echo "== craft gate references in repo =="
rg -n 'CI craft gates|Craft gates|Thread visualizer|thread visualizer|scripts/thread-visualizer-refresh' specs docs scripts 2>/dev/null || trueRepository: rdryfoos/HomesFlow
Length of output: 15486
Add a CI job for the thread visualizer refresh or remove it from the CI craft gates.
.github/workflows/ci.yml includes a craft-gate job, but no workflow runs scripts/thread-visualizer-refresh.sh. If this is a required PR gate, add a CI step that regenerates docs/thread-visualizer/data/ so stale data cannot merge silently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/001-mvp/craft-conventions.md` at line 94, Add CI coverage for the
Thread visualizer craft gate by updating the craft-gate workflow to run
scripts/thread-visualizer-refresh.sh and regenerate docs/thread-visualizer/data/
during validation, or remove the Thread visualizer entry from the craft
conventions if it is not required. Ensure the selected approach prevents stale
generated data from merging silently.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a read-only Thread Visualizer that projects Gate 2’s golden thread for demos: walker JSON (totals must match Gate 2) → single-requirement descent view → seeded “break the proof” state on
AC-USER-04.Traceability
Craft / demo tooling only (not product scope). Spec:
docs/thread-visualizer/SPEC.md.Proof
AC-USER-04includes PRD text,@coversmodules,test_AC_*--demo-break AC-USER-04yieldsGAPwith zero proof testsdocs/thread-visualizer/via local static server and click Break the proofHow to try
Summary by CodeRabbit
New Features
Documentation
Chores