Add guide and evals for state-aware sticky headers - #603
Conversation
a52b78f to
3112e18
Compare
3112e18 to
388701f
Compare
|
Results of local Pass Rate - Unguided: 0%, Guided: 0%{
"summary": {
"unguidedMedian": 0,
"guidedMedian": 0,
"unguidedPassRate": 0,
"guidedPassRate": 0,
"unguidedPassed": 0,
"unguidedTotal": 0,
"guidedPassed": 0,
"guidedTotal": 0,
"runsPerTest": 1,
"expectedTotalRuns": 1,
"taskCount": 1,
"runCountPerTask": 1,
"guideUsageRate": 0,
"guideUsageCount": 0,
"totalGuidedRuns": 1,
"totalGuidedNonDisciplineRuns": 1,
"toolActivationRate": 0,
"toolActivationCount": 0,
"unguidedEarlyFailures": 0,
"unguidedEarlyFailureRate": 0,
"guidedEarlyFailures": 0,
"guidedEarlyFailureRate": 0,
"guidedNonDisciplineEarlyFailures": 0
},
"results": {
"task - context-sensitive-sticky-headers - guided": [
{
"runNumber": 1,
"results": [],
"guidesUsed": [],
"retrievedGuides": [],
"fileReadGuides": [],
"guidanceToolsUsed": [],
"discipline": "user-experience",
"isSkill": false,
"expectedToolPrefixes": [
"modern-web"
],
"guideName": "context-sensitive-sticky-headers",
"taskName": "task",
"baseApp": "daily-grind",
"prompt": "Add new section headers inside the main content area of the page (e.g., within the cards or sections). These headers should stick to the top when scrolling."
}
],
"task - context-sensitive-sticky-headers - unguided": [
{
"runNumber": 1,
"results": [],
"guidesUsed": [],
"retrievedGuides": [],
"fileReadGuides": [],
"guidanceToolsUsed": [],
"discipline": "user-experience",
"isSkill": false,
"expectedToolPrefixes": [
"modern-web"
],
"guideName": "context-sensitive-sticky-headers",
"taskName": "task",
"baseApp": "daily-grind",
"prompt": "Add new section headers inside the main content area of the page (e.g., within the cards or sections). These headers should stick to the top when scrolling."
}
]
},
"stats": {
"task - context-sensitive-sticky-headers - unguided": {
"medianPassRate": 0,
"runPassRates": [
0
],
"runCount": 1,
"isSkill": false,
"passedChecks": 0,
"totalChecks": 0,
"earlyFailures": 0
},
"task - context-sensitive-sticky-headers - guided": {
"medianPassRate": 0,
"runPassRates": [
0
],
"runsUsingGuide": 0,
"runsWithToolActivation": 0,
"runCount": 1,
"isSkill": false,
"passedChecks": 0,
"totalChecks": 0,
"earlyFailures": 0
}
},
"timestamp": "2026-04-22T19:40:06.347Z",
"runCount": 1,
"agent": "gemini_cli",
"serving": "skills_cli",
"model": "gemini-pro-latest"
}Given the 0% guided pass rate, this use case would require some additional investigation and fine tuning. Let's remove the eval files from this PR to unblock getting the guidance in and eng will follow up on the evals separately. |
|
@LeaVerou are you able to take a look at this one? |
Just saw this, will take a look in a bit! |
SME ReviewP0: Flashing when slightly scrolledThe guide downplays the problem quite a bit:
This is what the demo page looks like if the container is only slightly scrolled: Screen.Recording.2026-05-01.at.20.46.43.movIt gets way worse if you also reduce the font-size (as is often needed). Sometimes the CSS will fight against the scrolling even: Screen.Recording.2026-05-04.at.09.24.05.movAnd even worse if you move the transition inside the CQ (so that it only applies when you go from unstuck → stuck but NOT when you go stuck → unstuck): Screen.Recording.2026-05-04.at.09.56.16.movYou can trigger the issue reliably by adding this snippet to the demo and playing with <script>
let firstHeader = document.querySelector(".section > .sticky-container");
firstHeader.scrollIntoView({behavior: "smooth"});
setTimeout(() => {
// Adjust this number for different versions of the problem. Any number between 1-43 seems to trigger it.
const offsetY = 10;
document.scrollingElement.scrollTop += offsetY;
}, 1000);
</script>Avoiding "large enough" changes does not fix this, it just reduces the interval where this is observable. I could not find any box-model affecting change where the problem was not present at all. Even going from I did end up getting badly nerd sniped by this and doing original research to come up with workarounds 😅, which I'll publish in a separate blog post (I can drop a link here when it's out if it's of interest), but so far all workarounds I found have their flaws. The best one so far is adding an Screen.Recording.2026-05-04.at.09.30.14.movFor this particular case, a Screen.Recording.2026-05-04.at.09.34.14.movBut a In both cases you'd want But we probably want to be recommending tried and tested techniques for this, not brand new workarounds… So, I'm not quite sure what's the best way forwards. 😞 P2: DO always specify container nameGiven that these CQs may be nested, we should emphasize that these queries should always include a container name. P2: Not sure what "context-sensitive" refers toI don't see anything context sensitive about this? The description doesn't help either:
Fallback strategiesP0: No fallback in the demo pageThe fallback is not present in the demo page. It should not only be there, but it should be verified that it's there via the expectations! P1: Discuss PE as the default and frame the
|
|
Just saw the review request — GitHub didn't bother notifying me for some reason. I'll try to take a look soon.
I held off on it because after talking to @tabatkins he thinks it's either a browser bug or a spec bug, so I filed w3c/csswg-drafts#13898 for now so we can figure that out first. |
Looks like |
Yes!! I was unsure about it being applied to It still worries me that it's disabling a supposedly positive behavior, and the lack of Safari support, but overall this does look very promising! The issue is also slotted to be discussed tomorrow, so I might have more data by then. |
LeaVerou
left a comment
There was a problem hiding this comment.
Only thing I'd consider blocking is the point about overflow-anchor — I'd really rather avoid applying it to root unless we really need to. In my testing it seems fine applied to the sticky parent: https://codepen.io/leaverou/pen/JobKgJQ
@patrickkettner curious if you came across cases that need it on the root?
|
@patrickkettner could you take a look at @LeaVerou's feedback? |
|
@rviscomi the latest push here is guide-only. I narrowed I'll re-run |
Co-authored-by: Lea Verou <lea@verou.me>
# Conflicts: # guides/scroll/context-sensitive-sticky-headers/demo.html # guides/scroll/context-sensitive-sticky-headers/guide.md # guides/user-experience/context-sensitive-sticky-headers/demo.html # guides/user-experience/state-aware-sticky-headers/demo.html
|
re-ran the evals on the current branch. 3 runs, gemini_cli (gemini-3.1-pro-preview) with skills_cli serving. all six agent executions completed, no early failures:
so current models will reach for scroll-state() unprompted most of the time; with the guide it was all three runs. Let me know if youd rather this be guidance only. Not sure why I was getting such different numbers from you before, but would love to compare evals.json if you arent seeing the same results. @rviscomi otherwise this is ready for another look. @LeaVerou the overflow-anchor scoping you flagged is in (sticky parent, not root, plus the bottom-stuck caveat). |
took her expectation rewording, dropped the IntersectionObserver bullets that contradicted the guide's progressive enhancement stance, and made the first task reduce padding when stuck so the overflow-anchor guidance actually gets exercised. the grader now checks overflow-anchor on the sticky parent both ways: required when stuck styles change layout, absent when they dont. also reverted unrelated dev-guide.ts changes that snuck in with an earlier merge.
typescript strictness fixes, height and line-height added to the layout prop list (the guide's own examples lead with height), a comment owning how conservative the list is, and scroll to the middle of the container instead of past it. recalibrated and re-graded the saved eval outputs, numbers unchanged.
|
Thanks Lea! Updated, re-ran the evals with the layout-changing task, guided 24/24, unguided 21/24 with all three misses being the missing overflow-anchor. |
|
We shouldn't need to update the README in this PR. Removing that should resolve the merge conflict, then we can push this change through. Edit: sorry I didn't realize the build script changed to auto-generate the README |
163b507 to
67817d4
Compare
67817d4 to
0dfe117
Compare
Fixes #256