Skip to content

refactor: migrate NOHIT classification to capability metadata#7775

Merged
walterbender merged 2 commits into
sugarlabs:masterfrom
Chaitu7032:refactor/block-capability-query
Jul 11, 2026
Merged

refactor: migrate NOHIT classification to capability metadata#7775
walterbender merged 2 commits into
sugarlabs:masterfrom
Chaitu7032:refactor/block-capability-query

Conversation

@Chaitu7032

Copy link
Copy Markdown
Contributor

summary

This PR moves the hidden and hiddennoflow block family from a global name list to block-owned capability metadata. The noHit capability is now declared next to the block definitions in FlowBlocks.js, stored through the existing ProtoBlock metadata path, and read by Block.isNoHitBlock() without changing the public helper API. Existing behavior stays the same, and the old list is still used only as a compatibility fallback when metadata is missing.

I also added tests to prove the new metadata path works, while keeping the old behavior covered:

  • capability present returns true
  • legacy fallback still works when metadata is absent
  • explicit false does not get overridden
  • normal blocks still return false

Manually verified no-hit behavior using a start → forever → drum stack. Disconnecting and reconnecting the drum block, docking interactions, and moving the complete stack continued to behave normally after migrating Block behavior to capability queries.

testing --

image Screenshot 2026-07-10 145428 Screenshot 2026-07-10 145400

PR category

  • feature
  • tests

@github-actions github-actions Bot added feature Adds new functionality tests Adds or updates test coverage size/M Medium: 50-249 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels Jul 10, 2026
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.21%. Comparing base (1267b95) to head (edb7f2b).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7775   +/-   ##
=======================================
  Coverage   58.21%   58.21%           
=======================================
  Files         174      174           
  Lines       57883    57887    +4     
=======================================
+ Hits        33696    33701    +5     
+ Misses      24187    24186    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Chaitu7032

Copy link
Copy Markdown
Contributor Author

@walterbender @vanshika2720

For this PR, i have started phase-2 .. I moved only the NOHIT classification to capability metadata. hidden and hiddennoflow now declare the noHit capability in their block definitions, and Block.isNoHitBlock() reads that metadata while keeping the legacy list as a fallback.

@vanshika2720

Copy link
Copy Markdown
Contributor

@Chaitu7032 looks good overall. Small suggestion: it might be worth adding a parity test between the legacy NOHIT list and the capability declarations. That would make future fallback removal safer and help prevent missing metadata entries.

@walterbender

Copy link
Copy Markdown
Member

Looks great. Maybe while you phase things in, you can leave a comment on the code that will be removed, e.g., Line 2136 in the updated version of block.js. (Or is there any reason to keep the old -- redundant -- mechanism is place?)

@Chaitu7032

Chaitu7032 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Looks great. Maybe while you phase things in, you can leave a comment on the code that will be removed, e.g., Line 2136 in the updated version of block.js. (Or is there any reason to keep the old -- redundant -- mechanism is place?)

Sure Sir , I will check whether there are any paths that still require the legacy fallback. If it’s no longer needed, I will remove it otherwise, I will add a comment explaining why it is temporarily kept. ...

Thanks for feedback . .

@Chaitu7032

Chaitu7032 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@Chaitu7032 looks good overall. Small suggestion: it might be worth adding a parity test between the legacy NOHIT list and the capability declarations. That would make future fallback removal safer and help prevent missing metadata entries.

Sure I will look into it .. 👍 thanks ..

@Chaitu7032

Copy link
Copy Markdown
Contributor Author

@walterbender

I looked into the old NOHIT list and found that we don't need the backup check anymore.

I removed the old list and the fallback code. Now, the system simply checks the new metadata directly on the hidden and hiddennoflow blocks. I also kept the tests to make sure this still works correctly.

Thanks for the suggestion .

@walterbender walterbender merged commit 4ce50ed into sugarlabs:master Jul 11, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files feature Adds new functionality size/M Medium: 50-249 lines changed tests Adds or updates test coverage

Projects

Development

Successfully merging this pull request may close these issues.

3 participants