chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

chore: update insta snapshot format - #5750

Merged
max-sixty merged 4 commits into
mainfrom
insta-update
Mar 27, 2026
Merged

chore: update insta snapshot format#5750
max-sixty merged 4 commits into
mainfrom
insta-update

Conversation

@snth

@snthsnth commented Mar 27, 2026

Copy link
Copy Markdown
Member

Force-update all integration test snapshots to current insta format. The newer insta (1.46.3) enforces a stricter snapshot format, causing CI to fail with 'existing value is in a legacy format'.

@snth
snth marked this pull request as draft March 27, 2026 15:20
snthand others added 2 commits March 27, 2026 17:37
Update inline snapshot string delimiters (@r" -> @", r### -> r#)
and remove deprecated snapshot_kind: text metadata from snap files.
All changes are format-only with no semantic differences.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Non-format changes from insta snapshot regeneration:
- compile/fmt append_select_nulls.snap: expression field corrected
to match current .prql file (stale '# duckdb:skip' / '# postgres:skip'
comments were removed from the file in a prior commit but the snap
metadata was never regenerated)
- compileall snap files: leading blank lines added to snapshot content.
Each blank line represents a dialect whose output matches the generic
dialect (empty diff). The previous insta version trimmed these; the
new version preserves them. The actual diff content is unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

This final commit c971aff is identical to 5f1de96 , I just split it into two commits to separate semantic changes from regenerating the snapshots to pure formatting changes. All tests passed for 5f1de96 but now some are failing for c971aff. What gives?

@max-sixty

Copy link
Copy Markdown
Member

great! will look at the snapshot issue

@prql-bot are you online?!

@prql-bot

Copy link
Copy Markdown
Collaborator

I'm online! 👋

Regarding @snth's question about the CI failure on c971aff — the failure is not related to the snapshot changes. It's a Docker Hub authentication error in the test-dbs-external job:

Head "https://registry-1.docker.io/v2/library/postgres/manifests/alpine": unauthorized: incorrect username or password

This is an infrastructure/credentials issue pulling the postgres container image. All the actual Rust tests, lints, builds, and code coverage checks passed successfully. A re-run of that job should fix it (assuming Docker Hub credentials are refreshed or the rate limit clears).

snthand others added 2 commits March 27, 2026 19:01
Authenticate with Docker Hub before pulling images to avoid rate limits.
- test-rust.yaml: login before docker compose up (postgres, mysql, clickhouse)
- build-devcontainer.yaml: login before image builds as defensive measure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Docker Hub login step in test-rust and build-devcontainer workflows
needs access to DOCKERHUB_USERNAME and DOCKERHUB_TOKEN secrets. These
are only available via 'secrets: inherit' on workflow_call invocations.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@snth
snth marked this pull request as ready for review March 27, 2026 17:43
@snth
snth requested a review from max-sixtyMarch 27, 2026 17:43
@snth

snth commented Mar 27, 2026

Copy link
Copy Markdown
MemberAuthor

@max-sixty I set up the Docker Hub account and added the details to the repo secrets. Should be good to merge now.

@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@max-sixty

Copy link
Copy Markdown
Member

thanks @snth !

@max-sixty
max-sixty merged commit a799b27 into mainMar 27, 2026
82 checks passed
@max-sixty
max-sixty deleted the insta-update branch March 27, 2026 19:44
@max-sixty

Copy link
Copy Markdown
Member

@prql-bot can you do a follow-up to change the code that produces those snapshots to avoid producing the blank lines at the top? fix at the source please

@prql-bot ?

prql-bot added a commit that referenced this pull request Mar 28, 2026
Skip empty diffs (where dialect output matches generic) instead of
including them, which produced leading/intermediate blank lines in
the snapshot files.
Closes follow-up from #5750.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prql-bot

Copy link
Copy Markdown
Collaborator

Done — opened #5752 to fix this at the source. The compileall test was collecting empty diffs (where a dialect's output matches generic) and joining them with newlines, producing the leading blank lines. The fix skips empty diffs entirely.

max-sixty pushed a commit to max-sixty/tend that referenced this pull request Mar 29, 2026
## Problem
The `tend-mention` workflow's `verify` job has a concurrency group keyed
by PR number with `cancel-in-progress: true`. When two comments arrive
on the same PR within seconds, the second run cancels the first — even
if the first was an explicit `@bot` mention.
This is exactly what happened at
[PRQL/prql#5750](PRQL/prql#5750 (comment)):
1. **19:44:09** — `@prql-bot can you do a follow-up...` → triggered run
[23664429862](https://github.com/PRQL/prql/actions/runs/23664429862)
2. **19:44:14** — `thanks @snth !` (5 seconds later) → triggered run
[23664431397](https://github.com/PRQL/prql/actions/runs/23664431397)
3. Run 2 **cancelled** run 1 via the concurrency group
4. Run 2 processed "thanks @snth" (no bot mention) → bot exited
silently. The explicit `@prql-bot` request was lost.
## Solution
- **Remove concurrency from the `verify` job** — it's lightweight and
fast; parallel verify jobs for the same PR are harmless.
- **Change `handle`'s `cancel-in-progress` to `false`** — jobs queue
instead of cancelling. The first handle completes (processes the
mention), then the second runs (exits silently for non-mention). No
mention gets dropped.
## Testing
- Added `test_mention_verify_no_concurrency` — verifies the verify job
has no concurrency group
- Added `test_mention_handle_job_queues_not_cancels` — verifies handle
uses `cancel-in-progress: false`
- Both tests fail before the fix, pass after
- Full suite: 110 tests pass, all pre-commit lints pass
---
Closes#93 — automated triage
---------
Co-authored-by: tend-agent <270458913+tend-agent@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@snth@max-sixty@prql-bot