fix(cascade): fuse heading-nav and TF-IDF signals instead of skipping TF-IDF on a zero-hit by tkcoding · Pull Request #137 · constructorfabric/studio

tkcoding · 2026-09-03T04:01:03Z

Summary

Addresses Problem 2 of JIT-retrieval cascade: auto-triggered OKF build and multi-signal fusion from #104 not implemented #134: route_tier1 escalated immediately whenever heading-nav found zero hits, without ever running TF-IDF — discarding a genuinely different signal. Heading-nav (find_sections) requires the query's exact literal substring to appear somewhere in a section's raw text; TF-IDF (score_sections) tokenizes on individual alphanumeric words, so a query differing from the source only in punctuation/spacing/hyphenation can still score unambiguously on TF-IDF even when heading-nav's exact-phrase match fails outright.
Both methods now always run, regardless of either one's own outcome. Two new Tier 1 rows cover heading-nav's zero-hit case:
- TF-IDF alone resolves at Tier 1 when its own signal is unambiguous (tfidf_only_unambiguous).
- Otherwise escalates, but hands along TF-IDF's own top pick as the Tier-2 candidate (heading_nav_no_hits_diffuse_tfidf) rather than none at all — unless TF-IDF also has no positive score anywhere, in which case it escalates with nothing (no_signal_from_either_method), same as the old behavior for a genuinely signal-less query.
Deliberately still out of scope: folding OKF concept-file summaries into Tier 1 as a fourth ranked signal. Tier 1's whole value is being free and deterministic without ever touching the OKF bundle; Tier 2 already consults OKF status once Tier 1 escalates.

Update: review round (ainetx) — 1 Major, 1 Minor, both fixed

Major, live-reproduced. When heading-nav found a hit but TF-IDF's top score was exactly 0 in every section (no real TF-IDF signal at all), route_tier1 still compared TF-IDF's document-order tie-break pick against heading-nav's real pick as if it were a genuine competing answer — fabricating a resolved_multi/heading_nav_tfidf_disagree verdict (or, by coincidence of order, a fabricated agreement) out of a signal that was never there. Fix: added an explicit new row (heading-nav has a hit, TF-IDF has no signal anywhere) that escalates on heading-nav's single, unconfirmed signal instead — symmetric to how the zero-hit rows already handle TF-IDF's own no-signal case. route_tier1 was split into two functions along the routing table's own two-part structure (heading-nav miss vs. hit) rather than disabling pylint's too-many-return-statements once the new row pushed past it. Added a regression test reproducing the review's exact repro case.
Minor. Five hand-built TestRouteTier2 fixtures still used the retired "heading_nav_no_hits" reason string (renamed to "no_signal_from_either_method" earlier in this same PR, without these being updated) — route_tier2 never reads the reason field, so nothing failed, but the drift was real and misleading. Fix: consolidated into one named constant instead of five separate literal copies, so a future rename can't silently skip these fixtures again. Test method names were also renumbered to match the routing table's real rows post-insertion (new row 4; disagree/agree/diffuse-margin shifted from 3/2/4 to 5/6/7).

Test plan

New fixtures (_TFIDF_ONLY_UNAMBIGUOUS_SAMPLE, _TFIDF_ONLY_DIFFUSE_SAMPLE) use a real hyphenation gap (source: "KAPING-framework" / query: "KAPING framework") to reproduce heading-nav's exact-phrase miss while TF-IDF's tokenizer still resolves it — hand-verified against tokenize()'s and _confidence()'s actual behavior, not just asserted against.
pytest tests/test_cascade.py -q — 30 passed (the only test file this PR's diff touches — an earlier version of this description incorrectly claimed a 5-file, 182-passed command pulled in from a broader sanity check, not this PR's own scope)
pylint on cascade.py — clean
cfs validate --artifact architecture/features/traceability-validation.md — passes (pre-existing, unrelated toc-missing-description warning only)
cfs spec-coverage --system studio --min-coverage 90 --min-file-coverage 60 --min-granularity 0.46 — all thresholds met
Independent of feat(cascade,doc-index): auto-trigger OKF build signal from real Tier-2 escalation counts #136 (Problem 1) — this branch is based directly on main, not stacked on that PR

Summary by CodeRabbit

Improvements
- Refined Tier 1 retrieval routing to combine heading navigation and TF-IDF results consistently.
- Improved handling of heading-navigation misses, unambiguous TF-IDF matches, weak signals, and ambiguous results.
- Preserved clear escalation behavior when retrieval confidence is insufficient.
Bug Fixes
- Prevented arbitrary zero-score results from being treated as meaningful disagreements.
- Improved handling of stale, missing, invalid, or incomplete retrieval data.

coderabbitai · 2026-09-03T04:01:10Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 3395d05e-0fd0-4720-97ae-09e9ff708109

📥 Commits

Reviewing files that changed from the base of the PR and between a84828d and d0d43a3.

📒 Files selected for processing (3)

architecture/features/traceability-validation.md
skills/studio/scripts/studio/utils/cascade.py
tests/test_cascade.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

Tier 1 now always runs heading navigation and TF-IDF scoring. The cascade adds seven routing outcomes, distinguishes zero-score TF-IDF results from real signals, updates Tier 2 escalation documentation, and expands regression coverage.

Changes

Tier 1 cascade routing

Layer / File(s)	Summary
Routing contract and implementation `architecture/features/traceability-validation.md`, `skills/studio/scripts/studio/utils/cascade.py`	The routing table expands to seven rows. `route_tier1` always runs TF-IDF scoring, handles heading-navigation misses through `_route_tier1_heading_nav_miss`, and avoids treating zero-score results as real disagreements.
Tier 2 escalation contract `skills/studio/scripts/studio/utils/cascade.py`, `tests/test_cascade.py`	The Tier 2 contract lists the new escalating rows. Tests cover candidate-scoped and full-bundle trust checks.
Routing matrix regression coverage `tests/test_cascade.py`	Tests cover all seven outcomes, exact reason strings, call isolation, TF-IDF-only results, heading-navigation candidates, and diffuse confidence.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to d0d43

Tier 1 now evaluates heading-navigation and TF-IDF together, resolving clear TF-IDF-only matches and escalating ambiguous or absent signals without fabricating zero-score disagreements. The documented routing behavior and regression coverage support merge readiness.

Sequence Diagram(s)

sequenceDiagram
  participant route_tier1
  participant heading_nav
  participant score_sections
  participant route_tier2
  participant OKF
  route_tier1->>heading_nav: obtain heading-navigation result
  route_tier1->>score_sections: score sections with TF-IDF
  score_sections-->>route_tier1: ranked results and signal status
  route_tier1->>route_tier1: resolve or create escalation result
  route_tier1->>route_tier2: pass escalating result
  route_tier2->>OKF: validate candidate or all sections
  OKF-->>route_tier2: return trust status

Suggested reviewers: ainetx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 2 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: Tier 1 now combines heading-nav and TF-IDF signals instead of skipping TF-IDF after a zero-hit.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 2 files. (1 skipped: 1 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

code-ranker-app · 2026-09-03T04:02:16Z

code-ranker

Built on a fork. View full report ↗

python

Metric	Baseline	Current	Δ
Complexity
cognitive — Cognitive complexity	113	113	$\color{#c0392b}{+0.034}$
cyclomatic — Cyclomatic complexity	114	114	$\color{#c0392b}{+0.041}$
Coupling
hk — God-object risk	1.5M	1.5M	$\color{#c0392b}{+7.8}$
Halstead
bugs — Estimated bugs	3.3	3.3	$\color{#c0392b}{+0.001}$
effort — Implementation effort	2M	2M	$\color{#c0392b}{+287}$
length — Total tokens	1902	1902	$\color{#c0392b}{+0.711}$
time — Coding time (s)	109.3K	109.3K	$\color{#c0392b}{+15.9}$
vocabulary — Distinct symbols	251	251	$\color{#c0392b}{+0.078}$
volume — Code volume	17.4K	17.4K	$\color{#c0392b}{+5.4}$
Lines of Code
blank — Blank lines	64.8	64.9	+0.031
cloc — Comment lines	112	112	+0.556
sloc — Source lines	407	408	+0.195
Maintainability
mi — Maintainability index	47	46.9	$\color{#c0392b}{-0.09}$
mi_sei — Maintainability (SEI)	42.7	42.6	$\color{#c0392b}{-0.138}$

… TF-IDF on a zero-hit Problem 2 of constructorfabric#134: route_tier1 escalated immediately whenever heading-nav found zero hits, without ever running TF-IDF -- discarding a genuinely different signal. Heading-nav requires the query's exact literal substring somewhere in a section's raw text; TF-IDF tokenizes on individual words, so a query differing from the source only in punctuation/spacing/hyphenation can score unambiguously on TF-IDF even when heading-nav's exact-phrase match fails outright. Both methods now always run. Two new Tier 1 rows cover heading-nav's zero-hit case: TF-IDF alone resolves at Tier 1 when unambiguous, and otherwise escalates with TF-IDF's own top pick as the Tier-2 candidate (rather than none at all) unless TF-IDF also has no positive score anywhere. Existing rows (agree/disagree/diffuse-margin when heading-nav does hit) are unchanged. Deliberately still out of scope: folding OKF summaries into Tier 1 as a fourth signal -- Tier 1's whole value is being free and deterministic without ever touching the OKF bundle. Signed-off-by: TECK KEAT WILSON <yeow.teck.keat@constructor.tech>

…signal Review findings on PR constructorfabric#137 (ainetx): - Major: route_tier1 unconditionally compared tfidf_ranked[0] against heading-nav's pick once heading-nav found a hit, even when every section scored exactly 0 (no real TF-IDF signal at all). tfidf_ranked[0] in that case is just an arbitrary document-order tie-break, not a genuine top pick -- comparing it anyway fabricated a "resolved_multi" / "heading_nav_tfidf_disagree" verdict (or, by coincidence of order, a fabricated agreement) out of a signal that was never there. Added an explicit row 4 (heading-nav has a hit, TF-IDF has no signal anywhere) that escalates on heading-nav's single, unconfirmed signal instead, symmetric to how rows 2/3 already handle TF-IDF's own no-signal case. Splits route_tier1 into two functions along the table's own two-part structure (heading-nav miss vs. hit) rather than disabling pylint's too-many-return-statements once the new row pushed past it. - Minor: five hand-built TestRouteTier2 fixtures still used the retired "heading_nav_no_hits" reason string (route_tier1 was renamed to "no_signal_from_either_method" for that case in the prior commit, without these being updated). Consolidated into one named constant, since route_tier2 never reads the reason field -- these fixtures only ever needed the row-1 *shape*, not a specific string -- so a future rename can't silently drift out of sync with these tests again. - Renamed the TestRouteTier1 test methods to match the routing table's real row numbers post-insertion (row 4 new; disagree/agree/diffuse-margin shifted from 3/2/4 to 5/6/7) and added a regression test reproducing the review's exact repro case for the new row 4. Signed-off-by: TECK KEAT WILSON <yeow.teck.keat@constructor.tech>

…ote TF-IDF's unconditional cost Review findings on PR constructorfabric#137 (ainetx, second pass against 6645603): - route_tier1's docstring enumerated candidate-count rows as "2/4/6/7" but its own parenthetical immediately after ("rows 3/4/7 despite escalating") named row 3 as one of the escalating-but-one-candidate rows -- contradicting the main list, which omitted it. Row 3 (heading- nav miss, TF-IDF diffuse) does return exactly one candidate, same as rows 2/4/6/7; the list was just wrong. Fixed to "2/3/4/6/7", so the list and its own parenthetical agree with each other and with the actual implementation. - Fusing heading-nav and TF-IDF (this function's whole point) means score_sections() now runs on every call, even one that a heading-nav hit could otherwise have resolved without it -- tfidf.py's own module docstring documents this scoring method as validated against a single ~166-page/~9-section document with deliberately no cap on section count or file size. That's an accepted, explicit tradeoff of fusing the signals, not an oversight; documented at both the call site and in route_tier1's own docstring rather than left implicit. Signed-off-by: TECK KEAT WILSON <yeow.teck.keat@constructor.tech>

…ce, and OKF independence Review findings on PR constructorfabric#137 (ainetx, second pass against 6645603): - No test exercised score_sections' single-section `unambiguous` code path: with only one ranked section, `_confidence` returns `unambiguous=True` purely because there's no second candidate to compare against (`len(ranked) == 1`), never by beating a real rival's score. Every existing row-2 fixture used two sections and only ever exercised the latter path. Added test_row2_tfidf_only_unambiguous_with_single_section_document with a genuinely single-section fixture, verified against tfidf.py's actual `_confidence` logic rather than asserted blindly. - No regression test guarded that the row-2 (tfidf_only_unambiguous) branch never falls through to route_tier2/OKF. Added test_row2_tfidf_only_unambiguous_never_calls_route_tier2_or_okf, spying on both with unittest.mock.patch and asserting neither is called. - route_tier1's OKF-independence guarantee (module docstring: Tier 1 must stay free and deterministic, never touching the OKF bundle) had no automated backing. Added test_route_tier1_never_touches_okf, which patches get_okf_status to raise if called at all and exercises every routing-table row (1 through 7, plus the margin_threshold opt-in) against it. - route_tier2 had no test for the new row-3 (heading-nav miss, TF-IDF diffuse) escalation, whose candidate is TF-IDF-sourced -- a provenance never exercised before (every prior route_tier2 test used a heading-nav-sourced candidate or none at all). Added test_row3_tfidf_sourced_candidate_recommends_okf_when_current, building the row-3 result via a real route_tier1 call so it breaks if row 3's actual shape ever changes. Signed-off-by: TECK KEAT WILSON <yeow.teck.keat@constructor.tech>

…er2, tighten row-3 assertion, enforce row-number docstring Third ainetx review pass on constructorfabric#137 found four more gaps: - test_row3_tfidf_sourced_candidate_recommends_okf_when_current wrote every section's concept file current, so a route_tier2 that (bug) validated the whole bundle instead of narrowing to just the named candidate would still pass. Added test_row3_tfidf_sourced_candidate_narrows_staleness_to_only_that_section, which only makes the candidate's own section current and leaves the other section missing -- verified it actually fails under that bug by injecting it and running the new test before reverting. - route_tier2's docstring claims it's "only called for the escalating rows: row 1 ... row 3 ... row 4, and row 7" but nothing enforced that cross-reference against route_tier1's real behavior. Added test_docstrings_row_number_cross_reference_matches_actual_escalating_reasons, which runs route_tier1 against a fixture for all seven table rows and asserts the escalating rows/reasons that actually come back are exactly what the docstring claims. - test_row3_tfidf_only_diffuse_escalates_with_tfidf_pick_as_candidate asserted tier/reason/candidates as three separate assertions instead of one whole-dict equality, unlike every other row test in this file -- switched it to match that pattern. - Row 4's own route_tier1 output was tested, but nothing fed a real row-4 result into route_tier2, unlike row 3. Added test_row4_heading_nav_sourced_candidate_recommends_okf_when_current, modeled on the existing row-3 route_tier2 test.

…er2, tighten row-3 assertion, enforce row-number docstring Third ainetx review pass on constructorfabric#137 found four more gaps: - test_row3_tfidf_sourced_candidate_recommends_okf_when_current wrote every section's concept file current, so a route_tier2 that (bug) validated the whole bundle instead of narrowing to just the named candidate would still pass. Added test_row3_tfidf_sourced_candidate_narrows_staleness_to_only_that_section, which only makes the candidate's own section current and leaves the other section missing -- verified it actually fails under that bug by injecting it and running the new test before reverting. - route_tier2's docstring claims it's "only called for the escalating rows: row 1 ... row 3 ... row 4, and row 7" but nothing enforced that cross-reference against route_tier1's real behavior. Added test_docstrings_row_number_cross_reference_matches_actual_escalating_reasons, which runs route_tier1 against a fixture for all seven table rows and asserts the escalating rows/reasons that actually come back are exactly what the docstring claims. - test_row3_tfidf_only_diffuse_escalates_with_tfidf_pick_as_candidate asserted tier/reason/candidates as three separate assertions instead of one whole-dict equality, unlike every other row test in this file -- switched it to match that pattern. - Row 4's own route_tier1 output was tested, but nothing fed a real row-4 result into route_tier2, unlike row 3. Added test_row4_heading_nav_sourced_candidate_recommends_okf_when_current, modeled on the existing row-3 route_tier2 test. Signed-off-by: TECK KEAT WILSON <yeow.teck.keat@constructor.tech>

tkcoding · 2026-09-07T09:44:15Z

Note: the round-3 fix commit referenced in the four replies above as d0d43a31 was missing its required DCO sign-off and got amended to ad30b6f4 (same diff, same message, just the trailer added) and force-pushed. Same content, new SHA.

sonarqubecloud · 2026-09-07T09:45:01Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ainetx

Automated review complete -- no blocking findings remain and CI is passing.

tkcoding force-pushed the fix/jit-retrieval-multi-signal-fusion-134 branch from 7b9a838 to af084c5 Compare September 3, 2026 04:03

ainetx reviewed Sep 3, 2026

View reviewed changes

Comment thread skills/studio/scripts/studio/utils/cascade.py Outdated

ainetx reviewed Sep 3, 2026

View reviewed changes

Comment thread skills/studio/scripts/studio/utils/cascade.py