feat: Jake's Resume template as default + /add command#647
Open
parthdiwane wants to merge 8 commits into
Open
Conversation
## Jake's Resume Template (default CV output) - Replace templates/cv-template.tex with Jake's Resume template (sb2nov/resume) — simpler, cleaner, widely recognized format - Section names updated: Experience (not Work Experience), Projects (not Personal Projects) — matches Jake's original - Remove fontawesome/multicol dependencies; template now works with any standard LaTeX installation - Default cv.output_format changed from "html" to "latex" in auto-pipeline.md and profile.example.yml - generate-latex.mjs: prefer tectonic, strip pdflatex-only primitives at compile time (upstream approach, no \ifpdf needed) - generate-latex.mjs: REQUIRED_SECTIONS updated to match new section names ## /add command - New modes/add.md: add a GitHub repo, project page, paper URL, or freeform description directly into cv.md + article-digest.md - Classifies input (project / experience / publication / skill) - Fetches GitHub API + README for repo URLs; Playwright for pages - Shows a preview and requires confirmation before writing - Duplicate detection: checks if entry already exists in cv.md - SKILL.md router: /career-ops add registered + discovery menu - AGENTS.md skill modes table updated ## Tests (test-all.mjs) - Section 11: /add command integrity (8 behavioral assertions) - Section 12: Jake's Resume template integrity (12 structural assertions: placeholders, section names, tectonic support, auto-pipeline default) - add.md added to expectedModes list - Total: 97 tests passing (was 65)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a ChangesAdd Command and LaTeX Default Template
Sequence DiagramsequenceDiagram
participant User
participant AddMode as /career-ops add
participant GitHubAPI as github.com API
participant Renderer as Playwright/Fetch
participant Extractor
participant Preview
participant CV as cv.md
participant Digest as article-digest.md
User->>AddMode: submit URL or text
AddMode->>GitHubAPI: fetch repo metadata & README (if github.com)
AddMode->>Renderer: render & fetch page (fallback)
GitHubAPI->>Extractor: provide content
Renderer->>Extractor: provide page HTML
Extractor->>Preview: structured fields + bullets
Preview->>User: show preview
User->>Preview: confirm / edit / skip
Preview->>CV: write entry (on confirm)
Preview->>Digest: append proof-point (on confirm)
CV->>User: post-write confirmation and run cv-sync-check
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 @.agents/skills/career-ops/SKILL.md:
- Line 6: The argument-hint string in SKILL.md currently includes an unsupported
"update" option; remove "update" from the argument-hint value (the line starting
with argument-hint: "[scan | deep | pdf | ...]") so the hint matches the actual
routed modes, or alternatively implement and document an "update" mode if
intended—but do not leave "update" advertised without a corresponding route or
documentation.
In `@AGENTS.md`:
- Line 213: Update the documentation mapping for the "add" mode so it lists all
supported item types (projects, GitHub repos, experiences, publications, and
skills) instead of only "project, GitHub repo, or experience"; locate the table
row that defines the `add` mapping and expand its right-hand description to
include "publication" and "skill" so the AGENTS.md routing/discovery cues match
the actual add workflow.
In `@modes/_shared.md`:
- Line 126: The tools table currently directs "Write" outputs to applications.md
which violates the tracker flow; update the table entry (the row containing
"Write | Generated .tex CV, applications.md, reports .md") to instead point
writers to "batch/tracker-additions/*.tsv" and note that merges should be
handled by merge-tracker.mjs (i.e., write TSVs to batch/tracker-additions/ and
run merge-tracker.mjs rather than editing applications.md directly).
In `@modes/add.md`:
- Around line 7-11: Add strict URL safety checks before any WebFetch/Playwright
navigation: in the add mode handler (the code path that accepts user input for
URLs—e.g., the function that calls WebFetch or Playwright navigation), parse the
incoming URL and allow only http/https schemes, then resolve the hostname to
IP(s) and reject any that are loopback (127.0.0.0/8, ::1), private/RFC1918
(10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), link-local (169.254.0.0/16,
fe80::/10), carrier/unique local (fc00::/7), or other non-routable ranges; also
reject literal IPv6/IPv4 internal addresses and hostnames like "localhost" or
other internal names before performing WebFetch or Playwright navigation. Ensure
the validation is applied in the same input handling used by both WebFetch and
Playwright navigation paths so all fetches are blocked for disallowed targets.
- Line 46: Several fenced code blocks in the document are missing language
identifiers and/or required blank-line separation which triggers markdownlint
rules MD040/MD031; find each triple-backtick fenced block (``` ) used for
examples/commands and add an appropriate language tag (e.g., ```bash, ```json,
```md, ```text) and ensure there is a blank line before and after fenced blocks
where needed. Update every occurrence referenced in the comment (the fenced
blocks around the examples/commands) so they all include a language identifier
and proper blank-line separation to satisfy markdownlint.
- Around line 31-34: The README fetch logic currently hardcodes raw URLs with
"main" or "master"; change it to use the branch-agnostic GitHub API: call GET
/repos/{owner}/{repo}/readme (or first GET /repos/{owner}/{repo} and use its
default_branch) instead of constructing
https://raw.githubusercontent.com/{owner}/{repo}/main/README.md; update the
steps in modes/add.md accordingly so the README retrieval follows the API
response (decode content from the /readme endpoint or build raw URL from
default_branch) to handle repositories whose default branch is not main/master.
In `@test-all.mjs`:
- Around line 445-449: The current check in test-all.mjs only inspects
tex.includes('\\usepackage{fontawesome}') and can miss reintroductions of other
banned packages; update the logic around tex.includes and the pass/warn branch
to search for all banned LaTeX packages (at least 'fontawesome', 'fontawesome5',
and 'multicol') by checking for their respective \usepackage strings, and if any
are present call warn(...) with a single message listing the offending packages,
otherwise call pass(...); locate the check using the tex variable and the
existing pass('cv-template.tex...') / warn('cv-template.tex...') calls to
modify.
- Around line 455-485: The tests currently skip checks if files are missing
(using fileExists before reading), which masks regressions; update the test
logic around fileExists('generate-latex.mjs') and
fileExists('modes/auto-pipeline.md') to explicitly fail when the expected files
are absent by invoking the existing fail(...) helper with clear messages (e.g.,
referencing generate-latex.mjs and modes/auto-pipeline.md), while keeping the
existing content checks that use readFile and variables genLatex and ap and the
pass(...) / fail(...) assertions intact.
- Around line 382-395: The test currently skips assertions when
'.claude/skills/career-ops/SKILL.md' is absent; update the file-existence check
around fileExists('.claude/skills/career-ops/SKILL.md') to explicitly fail the
test when the file is missing (e.g., call fail('SKILL.md is missing') or
similar) so the suite does not pass silently; keep existing logic that reads via
readFile(...) and the pass/fail checks for backticked 'add' and '/career-ops
add' intact.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3610b46-959d-469d-a273-b3e8817d3ab3
📒 Files selected for processing (10)
.agents/skills/career-ops/SKILL.mdAGENTS.mdconfig/profile.example.ymlgenerate-latex.mjsmodes/_shared.mdmodes/add.mdmodes/auto-pipeline.mdmodes/latex.mdtemplates/cv-template.textest-all.mjs
- modes/add.md: add URL safety validation (reject non-http/s, private/loopback IPs) - modes/add.md: use branch-agnostic GitHub API endpoint for README fetch - modes/add.md: add language tags to all fenced code blocks (MD040) + blank line separation (MD031) - AGENTS.md: expand /add routing description to include publications and skills - modes/_shared.md: fix Write row to reference TSV/merge-tracker pattern, not applications.md directly - .agents/skills/career-ops/SKILL.md: remove dead "update" from argument-hint - test-all.mjs: add fail() for missing SKILL.md, generate-latex.mjs, auto-pipeline.md (no more silent skips) - test-all.mjs: expand fontawesome ban to include fontawesome5 and multicol; use fail() not warn()
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test-all.mjs`:
- Around line 399-406: The test currently calls readFile('AGENTS.md') directly
which will throw if the file is missing; update the block to first check file
existence (using the same fileExists guard pattern used for SKILL.md checks)
before calling readFile('AGENTS.md'), and if fileExists('AGENTS.md') is false
then call fail('AGENTS.md missing') (or the existing fail style) so the test
fails gracefully; keep the existing content checks for '`add`' and
'project'/'GitHub' after reading the file.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 928c272b-f7c8-4951-85f2-5d57f085e5eb
📒 Files selected for processing (5)
.agents/skills/career-ops/SKILL.mdAGENTS.mdmodes/_shared.mdmodes/add.mdtest-all.mjs
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@test-all.mjs`:
- Around line 327-334: The test uses brittle hardcoded labels in the addSections
array (const addSections = ['Pipeline', 'Input Formats', 'Rules', 'Step 5',
'Step 6', 'Step 7']) which couples it to numbering; update the check in
test-all.mjs to use semantic section names instead of literal "Step X"
strings—replace 'Step 5', 'Step 6', 'Step 7' with descriptive identifiers (e.g.,
'Classification', 'Fetch Strategy', 'Generate Bullets' or whatever the actual
workflow stage names are) and ensure the loop that compares addSections to
addMode still uses the same logic (for (const section of addSections) { if
(addMode.includes(section)) ... }) so tests assert on meaningful headings rather
than numeric step labels.
- Around line 495-498: The current test uses a loose check ap.includes('latex')
&& ap.includes('default') which can give false positives; update the condition
in the test (the branch that calls pass('auto-pipeline.md uses latex as default
output format') / fail(...)) to assert a tighter relationship between "default"
and "latex" by using a regex or string proximity check on the ap content (e.g.,
test for a pattern like
/output[_\s-]*format.*default.*latex|default.*output[_\s-]*format.*latex/ or
ensure "latex" appears within N characters/lines of "default"), so the test only
passes when the configuration explicitly sets latex as the default output
format.
- Around line 358-362: The if condition checking addMode uses both
addMode.toLowerCase().includes('never invent') and addMode.includes('NEVER
invent'), which is redundant; simplify by keeping a single case-insensitive
check (e.g., use addMode.toLowerCase().includes('never invent') or a
case-insensitive regex like /never invent/i) and remove the duplicate
addMode.includes('NEVER invent') branch so the logic in the if/else (pass/fail)
remains unchanged.
- Around line 471-476: The validation for the Experience section in test-all.mjs
is over-escaped and inconsistent; simplify it by using a single clear check (for
example a regex or a plain substring) against genLatex that matches
"section{Experience}" regardless of quoting/escaping, so replace the multiple
includes with one sanity check (e.g., /section\{Experience\}/ or
genLatex.includes('section{Experience}')); target the test block referencing
genLatex and the generate-latex.mjs REQUIRED_SECTIONS logic to ensure the test
asserts the presence of "Experience" rather than matching specific escaped quote
patterns.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b20f6552-4a23-4031-b29d-00bc82b8ae6b
📒 Files selected for processing (1)
test-all.mjs
Comment on lines
+495
to
+498
| if (ap.includes('latex') && ap.includes('default')) { | ||
| pass('auto-pipeline.md uses latex as default output format'); | ||
| } else { | ||
| fail('auto-pipeline.md must default to latex output format'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Loose semantic check for default output format.
The condition ap.includes('latex') && ap.includes('default') will pass if both words appear anywhere in the file, even in unrelated contexts (e.g., "Use latex when not using default settings" would pass). This doesn't verify the actual configuration relationship. Consider checking for a more specific phrase like 'default' near 'latex' or matching a pattern like output_format.*latex or latex.*default.
🤖 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 `@test-all.mjs` around lines 495 - 498, The current test uses a loose check
ap.includes('latex') && ap.includes('default') which can give false positives;
update the condition in the test (the branch that calls pass('auto-pipeline.md
uses latex as default output format') / fail(...)) to assert a tighter
relationship between "default" and "latex" by using a regex or string proximity
check on the ap content (e.g., test for a pattern like
/output[_\s-]*format.*default.*latex|default.*output[_\s-]*format.*latex/ or
ensure "latex" appears within N characters/lines of "default"), so the test only
passes when the configuration explicitly sets latex as the default output
format.
…names; remove redundant condition
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test-all.mjs (1)
495-498:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTighten the latex-default assertion to avoid false positives.
ap.includes('latex') && ap.includes('default')can pass when those words are unrelated. Use a relationship-aware regex (same line/block).Suggested fix
- if (ap.includes('latex') && ap.includes('default')) { + if (/output[_\s-]*format[\s\S]{0,120}default[\s\S]{0,120}latex|default[\s\S]{0,120}output[_\s-]*format[\s\S]{0,120}latex/i.test(ap)) { pass('auto-pipeline.md uses latex as default output format'); } else { fail('auto-pipeline.md must default to latex output format'); }🤖 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 `@test-all.mjs` around lines 495 - 498, The current check uses ap.includes('latex') && ap.includes('default') which can match unrelated occurrences; replace that boolean includes check with a relationship-aware regex test on the ap string so "latex" and "default" must appear on the same line/near each other (e.g. test ap against a regex like /(?:latex.*default|default.*latex)/m) and keep the existing pass('auto-pipeline.md uses latex as default output format') / fail('auto-pipeline.md must default to latex output format') logic using that regex result; update the conditional that references the variable ap to use the regex test instead.
🤖 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 `@test-all.mjs`:
- Around line 448-463: The package-detection logic using simple string includes
on the tex variable misses optioned forms like \usepackage[...]{fontawesome5};
update the three checks to test tex with a regex that matches \usepackage
optionally followed by [options] before the {package} token (e.g. test for
patterns like \\usepackage\s*(?:\[[^\]]*\]\s*)?\{fontawesome\},
\\usepackage\s*(?:\[[^\]]*\]\s*)?\{fontawesome5\}, and
\\usepackage\s*(?:\[[^\]]*\]\s*)?\{multicol\}) so optioned uses are caught and
the pass/fail branches remain the same.
---
Duplicate comments:
In `@test-all.mjs`:
- Around line 495-498: The current check uses ap.includes('latex') &&
ap.includes('default') which can match unrelated occurrences; replace that
boolean includes check with a relationship-aware regex test on the ap string so
"latex" and "default" must appear on the same line/near each other (e.g. test ap
against a regex like /(?:latex.*default|default.*latex)/m) and keep the existing
pass('auto-pipeline.md uses latex as default output format') /
fail('auto-pipeline.md must default to latex output format') logic using that
regex result; update the conditional that references the variable ap to use the
regex test instead.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b79b9ee5-41e3-4c0a-9dd4-6f578d19e288
📒 Files selected for processing (1)
test-all.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test-all.mjs (1)
495-498:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a stricter default-output assertion for
auto-pipeline.md.Line 495 is still too permissive:
ap.includes('latex') && ap.includes('default')can pass unrelated prose. Assert an explicit relationship (regex/proximity) so this check validates the actual default behavior.🤖 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 `@test-all.mjs` around lines 495 - 498, The current check on variable ap is too permissive; replace the includes-based condition in test-all.mjs with a proximity/regex assertion that ensures "default" and "latex" appear together (e.g., within the same phrase) so it validates the actual default output relationship; specifically update the if that references ap, and the pass/fail branch, to test ap against a regex that matches patterns like "default ... latex" or "latex ... default" (case-insensitive) instead of separate ap.includes checks.
🤖 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 `@test-all.mjs`:
- Around line 351-355: The test currently succeeds if addMode contains any of
'confirm', 'Proceed', or 'preview', which allows false positives; update the
assertion so it requires an explicit pre-write confirmation token (e.g., require
addMode.includes('confirm') or addMode.includes('preview') but remove the
permissive 'Proceed' match) before calling pass('add.md requires confirmation
before writing'), otherwise call fail('add.md must require user confirmation
before writing to cv.md'); locate the conditional that checks addMode and adjust
the boolean logic accordingly around the addMode variable and the pass/fail
calls.
---
Duplicate comments:
In `@test-all.mjs`:
- Around line 495-498: The current check on variable ap is too permissive;
replace the includes-based condition in test-all.mjs with a proximity/regex
assertion that ensures "default" and "latex" appear together (e.g., within the
same phrase) so it validates the actual default output relationship;
specifically update the if that references ap, and the pass/fail branch, to test
ap against a regex that matches patterns like "default ... latex" or "latex ...
default" (case-insensitive) instead of separate ap.includes checks.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 85220106-ed8a-4149-80f7-56cda44e02ce
📒 Files selected for processing (1)
test-all.mjs
parthdiwane
force-pushed
the
feat/jake-resume-template-and-add-command
branch
from
4187740 to
aea1c35
Compare
Author
|
All potential issue and major findings from CodeRabbit have been resolved, please take a look at this pr when you can. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jake's Resume Template (default CV output)
/add command
Tests (test-all.mjs)
What does this PR do?
Adds a
/addcommand (/career-ops add <url-or-text>) that lets users permanentlyadd a GitHub repo, project page, research paper, or freeform description to their CV.
The command fetches content automatically (GitHub API + README, or Playwright for other
pages), generates ATS-style bullets from what is actually on the page, shows a preview,
and writes to
cv.md+article-digest.mdonly after the user confirms — so everyfuture CV generation includes the new entry without any manual editing.
Also replaces the default LaTeX CV template with Jake's Resume (sb2nov/resume) — a
simpler, widely recognized single-column format that works with tectonic or pdflatex,
removes fontawesome/multicol dependencies, and uses cleaner section names (Experience,
Projects) that are better recognized by ATS parsers
Related issue
Closes #646
Type of change
Checklist
node test-all.mjsand all tests passQuestions? Join the Discord for faster feedback.
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests