docs: add sampler CI/auth design doc as the global reference#266
docs: add sampler CI/auth design doc as the global reference#266michieldegezelle wants to merge 1 commit into
Conversation
Consolidated single-source-of-truth for the Liquid tests + Liquid sampler CI auth work: read-only/decoupled-writer design for tests, serialize/sole-writer design for the sampler, the production build across be_market -> lu_market -> nl_market pilots, real bugs found via live validation and code review, and this session's compact-diff v2 work (results register, vanished-output grouping, scope tier, visual-only tier, --from-zip) plus the 3-repo rollout mechanics (unpinned npm install, pull_request_review resolving from the default branch only, forking the reusable workflow per-caller for branch testing). See §10 for the most recent additions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
WalkthroughThe pull request adds a consolidated plan for Liquid authentication and partner-scoped sampler workflows, covering token ownership, staging execution, GitHub Actions integration, result reporting, compact-diff v2, pilot findings, rollout status, and cleanup sequencing. ChangesCI authentication and sampler
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CI_AUTH_SAMPLER_PLAN.md`:
- Around line 138-143: Update the fenced code blocks in CI_AUTH_SAMPLER_PLAN.md
for the results.zip tree listing and the gh pr edit re-trigger commands to
include language identifiers: use text for the listing and shell or bash for the
CLI commands.
- Around line 317-320: Update the steady-state bullet in the D7 section to mark
the “partner key has no TTL” premise as superseded or corrected, consistent with
the surrounding corrections. State that the partner API key expires after 24
hours but self-heals through digest matching and /refresh_api_key, while
preserving the conclusion that no routine manual work is required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 36431cef-82ce-4464-9450-d5ddfdcfa956
📒 Files selected for processing (1)
CI_AUTH_SAMPLER_PLAN.md
| ``` | ||
| results/ | ||
| liquid_comparison_report.html # the diff SUMMARY (1.9 KB when 0 changed; 1.84 MB at 32 changed) | ||
| sample_entry_ids.yml # the sampled entries | ||
| output/{account,reconciliation}_entries/<id>/{before,after}/{rendered_text.md,*.liquid,*.json,*.html} # 154 MB raw | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to fenced code blocks.
markdownlint flags both blocks (results.zip contents and the gh pr edit re-trigger commands) as missing a language tag. Use text for the tree listing and shell/bash for the CLI commands.
Also applies to: 244-247
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CI_AUTH_SAMPLER_PLAN.md` around lines 138 - 143, Update the fenced code
blocks in CI_AUTH_SAMPLER_PLAN.md for the results.zip tree listing and the gh pr
edit re-trigger commands to include language identifiers: use text for the
listing and shell or bash for the CLI commands.
Source: Linters/SAST tools
| - **Steady-state ("does this need any ongoing manual work?", resolved 2026-07-10): NO.** | ||
| `SF_BASIC_AUTH` is permanent (beta is confirmed permanent; alpha reuses the same value). The | ||
| partner key has no TTL and self-heals via write-back, so there is no routine per-run or | ||
| per-week/month manual step. Write-back authenticates as `REPO_ACCESS_TOKEN`, a **1-year PAT** |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale "no TTL" claim not reconciled with the correction above it.
This bullet still states "Steady-state ("does this need any ongoing manual work?", resolved 2026-07-10): NO." ... "The partner key has no TTL and self-heals via write-back, so there is no routine per-run or per-week/month manual step." This directly contradicts the correction a few lines above in the same D7 block: "CORRECTED (2026-07-15): the partner api_key DOES expire — TTL is 24h." The "no TTL" claim below was wrong and this contradicts the "sampler needs no cron" conclusion it drove; superseded by §8's decision. Unlike the other superseded claims in this section, this bullet isn't struck through or annotated as corrected, so a reader skimming just this line would take the disproven premise as current fact.
Consider adding a strikethrough/correction note here consistent with the rest of D7 (the corrected conclusion is still true — self-heals via digest-match on /refresh_api_key — but the "no TTL" reasoning is outdated).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CI_AUTH_SAMPLER_PLAN.md` around lines 317 - 320, Update the steady-state
bullet in the D7 section to mark the “partner key has no TTL” premise as
superseded or corrected, consistent with the surrounding corrections. State that
the partner API key expires after 24 hours but self-heals through digest
matching and /refresh_api_key, while preserving the conclusion that no routine
manual work is required.
1 participant
Summary
Adds
CI_AUTH_SAMPLER_PLAN.mdas a single, consolidated reference for the Liquid tests + Liquid sampler CI/auth work — covering:be_market→lu_marketpilots, incidents found and fixed along the way (§7, §9)resultsregister diff, vanished-output grouping, scope/dependency tier, visual-only tier with field-levelview.htmldiffing,--from-zip(see #265) — plus the 3-repo rollout mechanics learned (unpinnednpm installas the whole rollout mechanism,pull_request_reviewresolving the workflow file from the default branch only — not the PR branch, forking the shared reusable workflow per-caller to test an unmerged branch), two real bugs found and fixed across all copies (a TOCTOU race, a write-back validation gap), and two review findings deliberately left alone as documented tradeoffs rather than auto-patched.Docs-only, no code changes. Meant as the ongoing reference doc for this work over the coming weeks — not tied to any single PR's lifecycle.
🤖 Generated with Claude Code