Repository navigation
fix(workflow): define tracker status enum once in /outcome, normalise readers - #299
Conversation
MadsLorentzen
left a comment
There was a problem hiding this comment.
Verified before reviewing: full suite (205 green), lint/guards/version-check, and mutation checks on the new test file (reverting each touched command to master fails the right tests - outcome.md 6, html-report.md 2, gmail-sync.md 1, notion-sync.md 1). The core design is right and matches the #298 discussion: single vocabulary block in /outcome, canonical underscore forms for the writer, read-tolerance for both spellings so no existing tracker breaks, and the archive enum kept properly separate. That part is merge-quality work.
Two blockers before it can go in, both look like haste artifacts:
-
The CHANGELOG insertion corrupts the existing #286 entry. Your new entry replaces the opening line of "The robots gate did not fail closed" (master line 44), leaving that entry's whole body - the soft-200 and percent-decoding bullets - dangling mid-sentence under your fork heads-up. Diff your branch's CHANGELOG against master and you'll see it immediately. Ask: restore the deleted line and make the #298 entry self-contained above it.
-
The notion-sync fix stops one step short of the actual write path. You updated the create-time Status option list (Step 3), but Step 4's upsert still pushes the raw tracker value through. A pre-existing tracker holding legacy
no responserows will have that string written to Notion, which auto-creates a select option per unique string - so an existing database (which already carries the space-form option, and whose options Step 3.4 rightly says are never removed) ends up with both spellings as separate options, splitting closed applications across two filter buckets. That's the exact bug class this issue is about, reproduced one hop downstream. Ask: add a normalization instruction to Step 4 - map legacy space spellings to canonical before setting Status, never push a space form to Notion - and mention in the CHANGELOG's fork note that pre-existing space-form select options simply go unused (they're never auto-removed).
Two non-blocking nits, take or leave: apply.md Step 6b.2's "when every match holds a final status" check should cross-reference the new vocabulary block so "final" has an anchor there too; and the html-report both-spellings test assertions scan the whole file rather than the Step 1 bucket section (your interview_only check is properly section-scoped, so the pattern is already in the file).
One more thing, and it matters: #298 is @jakob1379's find, and the PR invitation in that thread was addressed to him. The open-source norm when someone else's invited work interests you is to ask in the thread first. I'll take the best fix for the repo regardless of author, but I've asked jakob to review this PR since the analysis is his - please treat his review comments as carrying the weight of the issue author.
|
Thanks for picking this up, and for reading the issue closely enough to keep the two enums apart. The shape is definitely what I had in mind: one vocabulary block in I verified the two blockers already raised and agree with both, so I won't repeat them. Three other things came out of testing your PR, one of which I think is more serious. 1. The block never says the space forms are the canonical valuesOpen and Final are both explicit lists, and
One clause in the block fixes it and lets
2. The vocabulary block splits Step 1's numbered listIt sits between item 1 and item 2 of It also silently truncates your own Nothing fails today because no test section-scopes Step 1 yet. The next one to do so passes against a stub. Moving the block below the 3. Dropping
|
|
Thanks for the thorough reviews — both blockers and every finding are addressed in the new commit (03f3385). Mads' blockers: the CHANGELOG restores the #286 entry's opening line with the #298 entry self-contained above it, and /notion-sync Step 4 now normalises legacy space spellings to the canonical underscore forms before setting the Status property — never pushing a space form to Notion, which would auto-create a duplicate select option and split closed applications across two buckets; the fork heads-up now notes that pre-existing space-form options simply go unused since Notion never auto-removes them. Jakob's findings: the vocabulary block now states the space spellings are the same values as the underscore forms, not separate statuses, equally Final, so no reader can land on "not Final, not Open, undefined"; the block moved below Step 1's closing --- as its own section, leaving Step 1's numbered list intact (section-scoped reads see all four items again); Open is derived by exclusion from the single explicit Final list; /html-report's bucket map gains a case-insensitive catch-all that maps unrecognised values to Rejected/Closed and names them once in the status breakdown; /apply Step 6b and /interview Step 0 anchor their final/open decisions to the block; and /gmail-sync + /html-report drop their local restatements. Tests were extended to pin each of these (equivalence, open-by-exclusion, block placement, Notion normalisation, apply/interview anchors), and the html-report bucket assertions are scoped to the Step 1 section. Verified the same way as before: full suite 210 green, lint/guards/version-check pass, and reverting each touched file to master fails the matching tests. |
… readers (MadsLorentzen#298) The tracker CSV status column had no single authoritative definition. Six command files restated it with inconsistent spellings, producing two concrete bugs: - /outcome Step 4 wrote o response and offer declined (spaces). /html-report normalised only the underscore forms, so those rows matched no bucket and were silently dropped from the rejection-rate denominator. - /gmail-sync Step 2 hardcoded the final-status set with space spellings, so a row written with underscores was never recognised as final and the sync kept chasing closed applications. - /html-report included interview_only in its tracker bucket map; that value belongs to the archive outcome.md Status: field, not the CSV status column. Fix: add a '## Tracker status vocabulary' block in /outcome (the only CSV writer) defining the canonical underscore spellings once. Every reader now references that block or explicitly lists both spelling forms as read-tolerance for existing trackers. /outcome Step 4 writes no_response and offer_declined. /html-report loses interview_only and gains offer declined as a read-tolerance variant. /notion-sync Step 3 Status select options are aligned to the canonical spellings. Pinned by tests/test_tracker_status_vocab.py (9 new cases following the DraftedMeansDraftedToEveryReader CASES-table pattern). All 205 tests pass. framework_version: 1.3.0 -> 1.3.1
…dsLorentzen#298) Follow-up to ca40df2, incorporating the maintainer and issue-author reviews. Blockers fixed: - CHANGELOG: the MadsLorentzen#298 entry had replaced the opening line of the MadsLorentzen#286 robots entry, leaving its body dangling under the new fork heads-up. Restored the deleted line and made the MadsLorentzen#298 entry self-contained above it (MadsLorentzen). - /notion-sync Step 4 now normalises legacy space spellings to the canonical underscore forms before setting the Status property. A raw push would auto-create a separate Notion select option per unique string, splitting closed applications across two filter buckets in an existing database (MadsLorentzen). Issue-author findings: - The vocabulary block now states that the space spellings are the same values as the underscore forms, not separate statuses, equally Final. Previously a reader applying the Open/Final lists literally landed on "not Final, not Open, undefined" for `offer declined`, and /apply Step 6b would refresh a closed application's row instead of appending (jakob1379). - The block moved below Step 1's closing --- as its own section: it was splitting Step 1's numbered list and silently truncating section-scoped reads of Step 1 to item 1 (jakob1379). - Open is derived by exclusion from the one explicit Final list, so a new status needs updating in a single place (jakob1379). - /html-report's bucket map gains a case-insensitive catch-all that maps unrecognised values to Rejected/Closed and names them once in the status breakdown - the MadsLorentzen#298 failure mode with a different input (jakob1379). - /apply Step 6b and /interview Step 0 anchor their final/open decisions to the vocabulary block (jakob1379). - /gmail-sync and /html-report drop their local restatements of the read-tolerance rule (jakob1379). Tests: html-report bucket assertions scoped to the Step 1 section; new pins for the equivalence clause, open-by-exclusion, block placement, the Notion normalisation, and the apply/interview anchors.
03f3385 to
73abb56
Compare
|
Merged - and the rework was exactly what both reviews asked, verified before merge: all three of @jakob1379's findings addressed (the equivalence clause, the block moved out of Step 1's list - section-scoped reads see all four items again - and the case-insensitive catch-all), both of my blockers fixed (the #286 CHANGELOG entry restored, notion-sync normalising on create and update), 210 tests green, and my mutation checks fail one test each for the three new pins. I rebased the branch onto master before merging (the v1.4.0 cut had moved your CHANGELOG target section - your entry now opens a fresh [Unreleased]). Credit where due: the diagnosis and acceptance criteria are jakob's (#298), the data-loss find in apply.md's append-vs-update rule is jakob's review, and the implementation with the read-tolerance improvement is yours. Good collaboration - this is the repo working as intended. |
… readers (MadsLorentzen#299) * fix(workflow): define tracker status enum once in /outcome, normalise readers (MadsLorentzen#298) The tracker CSV status column had no single authoritative definition. Six command files restated it with inconsistent spellings, producing two concrete bugs: - /outcome Step 4 wrote o response and offer declined (spaces). /html-report normalised only the underscore forms, so those rows matched no bucket and were silently dropped from the rejection-rate denominator. - /gmail-sync Step 2 hardcoded the final-status set with space spellings, so a row written with underscores was never recognised as final and the sync kept chasing closed applications. - /html-report included interview_only in its tracker bucket map; that value belongs to the archive outcome.md Status: field, not the CSV status column. Fix: add a '## Tracker status vocabulary' block in /outcome (the only CSV writer) defining the canonical underscore spellings once. Every reader now references that block or explicitly lists both spelling forms as read-tolerance for existing trackers. /outcome Step 4 writes no_response and offer_declined. /html-report loses interview_only and gains offer declined as a read-tolerance variant. /notion-sync Step 3 Status select options are aligned to the canonical spellings. Pinned by tests/test_tracker_status_vocab.py (9 new cases following the DraftedMeansDraftedToEveryReader CASES-table pattern). All 205 tests pass. framework_version: 1.3.0 -> 1.3.1 * fix(workflow): address review findings on the tracker status enum (MadsLorentzen#298) Follow-up to ca40df2, incorporating the maintainer and issue-author reviews. Blockers fixed: - CHANGELOG: the MadsLorentzen#298 entry had replaced the opening line of the MadsLorentzen#286 robots entry, leaving its body dangling under the new fork heads-up. Restored the deleted line and made the MadsLorentzen#298 entry self-contained above it (MadsLorentzen). - /notion-sync Step 4 now normalises legacy space spellings to the canonical underscore forms before setting the Status property. A raw push would auto-create a separate Notion select option per unique string, splitting closed applications across two filter buckets in an existing database (MadsLorentzen). Issue-author findings: - The vocabulary block now states that the space spellings are the same values as the underscore forms, not separate statuses, equally Final. Previously a reader applying the Open/Final lists literally landed on "not Final, not Open, undefined" for `offer declined`, and /apply Step 6b would refresh a closed application's row instead of appending (jakob1379). - The block moved below Step 1's closing --- as its own section: it was splitting Step 1's numbered list and silently truncating section-scoped reads of Step 1 to item 1 (jakob1379). - Open is derived by exclusion from the one explicit Final list, so a new status needs updating in a single place (jakob1379). - /html-report's bucket map gains a case-insensitive catch-all that maps unrecognised values to Rejected/Closed and names them once in the status breakdown - the MadsLorentzen#298 failure mode with a different input (jakob1379). - /apply Step 6b and /interview Step 0 anchor their final/open decisions to the vocabulary block (jakob1379). - /gmail-sync and /html-report drop their local restatements of the read-tolerance rule (jakob1379). Tests: html-report bucket assertions scoped to the Step 1 section; new pins for the equivalence clause, open-by-exclusion, block placement, the Notion normalisation, and the apply/interview anchors.
… readers (MadsLorentzen#299) Cherry-picked from upstream (MadsLorentzen/ai-job-search e09d3eb). The tracker CSV status column had no single authoritative definition - six command files restated it with inconsistent spellings, producing two concrete bugs: /outcome Step 4 wrote "no response"/"offer declined" (spaces), which /html-report's bucket map (underscore-only) silently dropped from the rejection-rate denominator; /gmail-sync hardcoded the final-status set with space spellings, so an underscore-written row was never recognised as final and the sync kept chasing closed applications. A new "Tracker status vocabulary" section in /outcome now defines the canonical spellings once; every reader (html-report, gmail-sync, interview, notion-sync, apply) references that block instead of restating it. /html-report also gains a case-insensitive catch-all (unrecognised value -> Rejected/Closed, named once in the breakdown) instead of silently dropping it. Applied first in this pass because c855e11 (deadline persistence) and 762d321 (html-report deadline column) both reference this section by name - cherry-picking either of those first would have left a dangling "see Tracker status vocabulary above/below" reference. Fork-specific adaptation: this fork's real tracker already carries `expired` (13 rows) and `skipped` (4 rows) statuses that don't exist in upstream's vocabulary at all - discovered by checking the actual tracker data before merging. Folded both into the Final list (expired mirrors /rank's seen_jobs.json status for a dead posting; skipped marks a fully-evaluated role deliberately not pursued) since real data already uses them and neither implies a submission happened. Without this, outcome.md's "list open applications" would have started surfacing already-closed-out expired/skipped rows as needing follow-up action. Also discovered and fixed in passing: html-report.md's bucket map currently has no catch-all, so expired/skipped rows fall through unbucketed today - this cherry-pick's catch-all fixes that same-session gap. Verified: new tests/test_tracker_status_vocab.py 14/14 pass, full suite 244/244 pass.
* 'master' of https://github.com/Omar12/ai-job-search: (51 commits) docs(evaluation): reframe onboarding and matching guidance around function, not title (MadsLorentzen#330) fix(html-report): read and render the tracker deadline column (MadsLorentzen#325) fix(tracker): persist the application deadline end to end (MadsLorentzen#319) (MadsLorentzen#324) docs(changelog): the \namefont override is live on every moderncv version, not a 2.4+ no-op fix(cv-template): compile main_example.tex on apt-packaged moderncv (MadsLorentzen#242) (MadsLorentzen#323) docs(contributing): invited PRs are reserved for the invitee for a stated window fix(convert-salary-excel): reject ambiguous dot thousands separators (MadsLorentzen#326) chore(release): cut v1.5.0 feat(upstream): add commit-level triage and weekly watch for forks (MadsLorentzen#305) (MadsLorentzen#320) fix(security): gitignore /upskill reports at the path the skill writes them (MadsLorentzen#317) docs(scraper): drop the phantom 'evaluated' status from seen_jobs.json vocabulary (MadsLorentzen#316) docs(add-portal): honest-UA fetching contract line; relocate MadsLorentzen#304 entry to Unreleased docs(add-portal): specify credential handling for portals needing an API token (MadsLorentzen#304) feat(security): hold .claude/settings.json hooks to a reviewed allowlist (MadsLorentzen#313) docs(changelog): entry for MadsLorentzen#310 dynamic CI portal-CLI discovery ci: discover portal CLIs dynamically so fork-added portals get checked (MadsLorentzen#310) fix(apply): archive the job posting while /apply still holds it (MadsLorentzen#306) (MadsLorentzen#307) fix(workflow): define tracker status enum once in /outcome, normalise readers (MadsLorentzen#299) docs(changelog): cut v1.4.0 fix(cli): honest User-Agent token on linkedin-search ...
Problem
The tracker CSV
statuscolumn has no single authoritative definition. Six command files restate it independently with inconsistent spellings, producing two concrete bugs that reproduce onmaster @ 20d9507.Bug 1 — space form silently dropped from /html-report stats
/outcomeStep 4 wroteno responseandoffer declined(with spaces)./html-reportStep 1 normalisedno_response/offer_declined(underscores). Neither space form matched any bucket, so any row written by/outcomewith those final statuses was silently excluded from the rejection-rate denominator.Reproduction on the real path:
/outcome <company>→ Step 4 advanced the row's status tooffer declined(the form its own spec text stated at:126)./html-report→ Status normalisation at line 30 found no match → the row was absent from every bucket and from the Total-with-a-resolved-status count → rejection rate computed over a denominator that quietly excluded it.Bug 2 — underscore form never recognised as final by /gmail-sync
/gmail-syncStep 2 defined its "open application" set by excludinghired,rejected,no response,offer declined,withdrawn(space forms). A row written with the underscore forms — whether by a fork or bygmail-sync.md:74's own table, which already usedoffer_declined— was never recognised as final and the sync kept chasing a closed application.Bug 3 — archive enum value in tracker bucket map
/html-report:30listedinterview_onlyin its tracker bucket map.interview_onlyis a value of the archiveoutcome.mdStatus:field (documents/README.md:141), not a tracker CSV value. It has never been written to the CSV by any command in this repo.Fix
Define the enum once in
/outcome(the only writer of the tracker CSV), following the precedent/applyStep 6b already set with the CSV header.Changes
.claude/commands/outcome.md## Tracker status vocabularyblock after the header; update Step 1.3 to reference it; update Step 2 heading to distinguish archive enum; update Step 4 to write canonical underscore forms.claude/commands/gmail-sync.md.claude/commands/html-report.mdinterview_only; gainsoffer declinedas read-tolerance alongside canonicaloffer_declined; adds reference to vocabulary block.claude/commands/notion-sync.mdno responsetono_response, andoffer_declinedadded.claude/skills/job-application-assistant/SKILL.mdframework_version: 1.3.0→1.3.1tests/test_tracker_status_vocab.pyDraftedMeansDraftedToEveryReaderCASES-table patternCHANGELOG.md[Unreleased] ### FixedRead-tolerance
Every reader accepts both underscore and space spellings on read. No migration of existing trackers is needed.
/outcomeStep 4 now writes the canonical underscore forms. The vocabulary block states the read-tolerance rule explicitly so a future reader knows both spellings must be handled.interview_onlyRemoved from the
/html-reportbucket map only. Its definition indocuments/README.mdandoutcome.md's archiveStatus:field is untouched — that enum is out of scope per the issue discussion.Verification
Baseline on
masterbefore this branch: 196 tests. This PR adds 9 new cases.Acceptance criteria (from the issue)
interview_onlyremoved from the tracker bucket mapoffer declinedby/outcomereaches Rejected/Closed in/html-report(both spellings now listed in that bucket)tests/pins it, following the CASES-table pattern ofDraftedMeansDraftedToEveryReaderpython3 tools/lint_skills.py,python3 tools/check_framework_version.py,python3 tools/security_guards.py,python3 -m unittest discover -s testsall pass