changelog.d: four fragments on master do not end in a newline, so the next appended entry concatenates onto the previous bullet - #351
Conversation
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 27 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (5 files)
Reviewed by step-3.7-flash · Input: 28.2K · Output: 3.3K · Cached: 213.5K |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
|
MERGE. The fix is complete, it changes nothing but the trailing byte, and the enumeration was checked in both directions. Re-measured on a trial merge of The defect, and that the fix covers all of itThe card names four fragments. Rather than take that list as the scope, I enumerated every fragment in Found-not-claimed: none. Claimed-not-found: none. The four the card named are exactly the four that exist, and after the merge 0 of 28 fragments lack a trailing newline. The new fragment this PR adds ends It changes only the trailing byteA Gates and suite
One non-blocking accuracy note on the new fragment's wordingThe fragment this PR adds justifies the fix like this:
That consequence does not exist in this repository today. There is no changelog assembly step at all: no The fix is still correct on the grounds the card actually establishes — the house norm, measured at 38 of 40 Interaction with #352These two were flagged as possibly colliding, since Stated limitations
|
…of/review_by coercion guards forward with tests (#347) Revision of the closed #312. All three blockers from tsk-agi5ph resolved, both non-blocking items addressed, and the tests verified red-first rather than taken on the body's word. RED FIRST, this PR's test files run UNCHANGED against #307 (swap proven by md5 first: test_api.py d2736eae and test_collections_ingest.py c5787bc1 identical on both sides; api.py ce0ac0e0 (#307) vs ccc68390 (#347) genuinely differs): the card's five-case table, on #307: FAILED coerce_iso_string_timestamp_returns_hits ValueError FAILED coerce_int_review_by_returns_hits TypeError '<' int/str FAILED coerce_list_review_by_returns_hits TypeError '<' list/str coerce_str_review_by_overdue_is_past_review passed coerce_plain_metadata_returns_hits passed 3 failed, 2 passed -- failure-for-failure identical to the card's prediction all six coercion tests on #307: 4 failed, 2 passed -- exactly the body's "4 of 6" claim BLOCKER 2, the dropped producer, restored rather than rebuilt: taosmd/collections.py is byte-identical to #307 (md5 744af0bb on both). The producer is genuinely absent on master -- the end-to-end test cannot even import there (ImportError: cannot import name '_parse_front_matter'), so it discriminates. indexed_at is written at collections.py:909. The PR body states the chosen direction explicitly, as acceptance required. BLOCKER 3: the false UnboundLocalError changelog entry is gone; replacement fragment ends 0x0a. Guards are now symmetric (api.py:486 as_of, api.py:494 review_by). Gates clean (deleted-symbols, normalise-handle, witness), no conflict markers, no Removes-Intentionally trailer. Suite 1617 passed, 12 skipped -- reconciles as the post-#351 baseline 1591 + the 26 tests added. The card's bar was >1435. STATED LIMITATIONS. - test_search_hit_metadata_always_has_doc_currency_fields PASSES on #307 too. It is a valid output-shape contract guard but is NOT evidence anything was fixed; the discriminating tests are the four coercion cases. - The red-first baseline is #307, not master: against master these tests cannot import at all, so master is useless as a red baseline for the producer half. - The PR body reports no suite count, which the card's acceptance asked for in as many words. Ran it here rather than bounce correct work. This is the sixth PR in that pattern (#340, #341, #345, #346, #348, #347) and is worth fixing at the source. - api.py:498 builds is_past_review in a 110-character expression. No line-length rule exists in the repo, so style only; no change requested.
…tab lines name the canonical in-repo script (#354) Card tsk-bse2el. Replaces the hardcoded /home/jay/.taos-fleet-tools/resume_arm_time.py in system_crontab_block and do_fire with _HELPER_PATH = os.path.realpath(__file__). WHY THIS MATTERED MORE THAN THE CARD SAID The out-of-repo path the crontab lines named is not just a different copy, it is a stale pre-#337 one: md5 7c54e86e32 / 40843 bytes vs the repo's 446505544c / 42188 bytes, 45 lines of divergence, and its do_fire has NO A2A bus posting at all. On master an armed resume fired, wrote a log line, and notified nobody. Verified on a TRIAL MERGE into origin/master dddbb5d (merge-base re-derived as 72ade2c after #355 moved master mid-review), never on the PR head. - Real CLI (`resume_arm_time.py 2026-08-18T07:00:00+00:00`, read-only): master emits the .taos-fleet-tools path; the trial merge emits the invoking checkout's own script. - Mutation analysis: against the OLD impl the 4 touched tests all fail, but every one fails with `AttributeError: no attribute '_HELPER_PATH'`, which only proves the symbol is new. Against a MUTANT that DEFINES _HELPER_PATH but hardcodes it back, only 2 fail: test_do_fire_posts_resume_due_to_bus FAILED <- discriminates test_do_fire_bus_failure_writes_visible_record FAILED <- discriminates test_system_crontab_block_names_usr_bin_python3 PASSED <- vacuous wrt this defect test_do_fire_runs_as_subprocess PASSED <- vacuous wrt this defect The discriminating pair anchors on SCRIPT, an origin independent of the implementation. Followed up as tsk-vqfjsm; genuine coverage exists, so not a blocker. - Changelog fragment ends 0x0a, so it does not reintroduce the #351 defect. - Conflict markers, deleted-symbols guard, handle gate, witness gate: clean. - Full suite on the trial merge: 1617 passed, 12 skipped. Reconciles as 1617 = 1617 + 0. STATED LIMITATIONS - The emitted path now depends on WHERE the script is run. From a worktree or temp copy it pins a path that will be deleted; since the line is `<helper> --fire ... && (crontab -l | grep -v ...) | crontab -`, a missing helper short-circuits the && and the self-removal never runs, turning a one-shot into a permanent annual entry. Filed as tsk-bxdfvz. - Nothing here reconciles the two copies. The stale .taos-fleet-tools file is still on disk and still 45 lines behind; this only stops the repo copy pointing at it. Untracked. - os.path.realpath resolves symlinks, so a symlink install would be pinned by its target. No such install exists today. - My first suite run on this PR reported 1 failure. That was my own contaminated worktree (I reverted the tree mid-run), not the PR. Rebuilt, proved impl+tests by md5 before and after, re-ran clean. Recorded on the PR.
… NOT YET PINNED (#357) Card tsk-r44fqf. The README pinned a byte size and sha256 for longmemeval_s_full.json that nothing on this box can verify. This retains the claimed values, labels them unverified, and shows the stat/shasum commands a future reader should run -- matching the style already used for longmemeval_s_cleaned.json. Verified on a TRIAL MERGE into origin/master 10d60ab (merge-base re-derived as e759336 after #355 and #354 both moved master during the review), never on the PR head. - Card premise holds: `find /home/jay -name longmemeval_s_full.json` returns 0 hits. The README also claims a canonical copy on the project bench host, so I tried to settle the pins rather than hedge them: ssh to the bench host TIMED OUT. The values genuinely cannot be confirmed here. - Arithmetic re-derived on the merged text: 277383467 bytes -> 264.53 MiB vs claimed 265 MiB OK; 15388478 bytes -> 14.68 MiB vs claimed 14.7 MiB OK. - This file has produced three invented-precision defects (#339, #342, tsk-7cl7rk). This PR does the opposite: it retains the claimed values and labels them, rather than inventing replacements. - Conflict markers, deleted-symbols guard, handle gate, witness gate: clean. - Changelog fragment ends 0x0a; does not reintroduce the #351 defect. - Full suite: 1617 passed, 12 skipped. Reconciles as 1617 = 1617 + 0 (docs-only). STATED LIMITATIONS - "NOT YET PINNED" reflects this box plus one failed reachability check, NOT proof the pins are wrong. If the bench-host copy exists the values may be correct and merely unconfirmed. The wording is scoped to "this machine" and does not overclaim. - The `# expect: <sha>` line is removed from the verify snippet. Defensible while unpinned, and the sha is still stated above it, but a reader copying only the code block loses it inline. - Does NOT fix the missing trailing newline on this same README (last byte still 0x2a after this merge). That is tsk-lkctqr's item. - The PR body's file list is FABRICATED: it claims 24 files / 1863 insertions (a list belonging to #349's mentions work); the real diff is 2 files / 13 insertions. The diff is correct and is what was reviewed. Seventh PR in this write-up pattern.
1 participant
CARD TITLE (intent, not commit subject): changelog.d: four fragments on master do not end in a newline, so the next appended entry concatenates onto the previous bullet
Autonomous build of board card tsk-v2pr2b.
Files:
changelog.d/tsk-35fsll-eventqa-exit-code.md | 2 +-
changelog.d/tsk-cwzaab-fix-quoted-error-strings.md | 2 +-
changelog.d/tsk-nup2ny-fix-possessive-apostrophe.md | 2 +-
changelog.d/tsk-ocjut3-stream-rejection-buffer.md | 2 +-
changelog.d/tsk-v2pr2b-fix-missing-changelog-newlines.md | 2 ++
5 files changed, 6 insertions(+), 4 deletions(-)