fix(applications): address code review issues#429
Open
benzntech wants to merge 3 commits into
Open
Conversation
…asy_apply_submit Adds job-application FastMCP tools to the personal nfsarch33 fork: - save_job: bookmark a posting on LinkedIn (write, confirm_send-gated). - list_applied: read /my-items/saved-jobs/?cardType=APPLIED for the user's applied-jobs history with extracted job IDs to reconcile against the cashflow ledger. - easy_apply_inspect: open the Easy Apply dialog read-only and report step_count + question schema without submitting. - easy_apply_submit: submit only zero-question, single-step Easy Apply postings; multi-step or question-bearing postings return status=manual_review with the detected questions for human follow-up. All write actions follow the send_message pattern: confirm_send=False returns a structured preview, confirm_send=True performs the action. Detection uses structural DOM signals (aria-pressed, role=dialog, aria-label presence) rather than locale-dependent verb text, per the project's locale-independence rule. Includes unit tests for the four new tools (preview, confirm, manual review, list_applied) and adds them to the global timeout coverage. Sentrux baseline saved at .sentrux/baseline.json (Quality 6104, coupling 0.51, 0 cycles, 0 god files).
- Issue 1: Change destructiveHint to True for save_job - Issue 2: Add per-locale table for Easy Apply/Applied labels - Issue 3: Wait for confirmation after submit (dialog close or badge) - Issue 4: Remove b.type==='submit' fallback (risky for multi-step)
Contributor
Greptile SummaryThis PR addresses four code-review issues from PR #425: sets
Confidence Score: 3/5The submit confirmation wait introduced by this PR never actually waits — it snapshots the DOM synchronously and almost always reports submission_unconfirmed on real LinkedIn pages. The JavaScript polling loop in easy_apply_submit exits on the first iteration in every modern browser because window.Promise is always defined, meaning the tool will report submission_unconfirmed for most real submissions even when the apply succeeded. Separately, _upload_resume_to_form returns true without actually attaching any file, silently dropping resumes when easy_apply_full is eventually wired up. linkedin_mcp_server/scraping/extractor.py — the confirmation-wait JavaScript in easy_apply_submit and easy_apply_full, and the _upload_resume_to_form implementation. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant applications.py
participant extractor.py
participant LinkedIn
Client->>applications.py: easy_apply_submit(job_id, confirm_send=True)
applications.py->>extractor.py: easy_apply_submit(job_id, confirm_send=True)
extractor.py->>extractor.py: _open_easy_apply_dialog(job_id)
extractor.py->>LinkedIn: navigate to /jobs/view/{job_id}/
LinkedIn-->>extractor.py: page loaded
extractor.py->>LinkedIn: page.evaluate() detect Easy Apply button
LinkedIn-->>extractor.py: {has_easy_apply, already_applied}
extractor.py->>LinkedIn: page.evaluate() click Easy Apply
LinkedIn-->>extractor.py: dialog opened
extractor.py->>extractor.py: _inspect_easy_apply_dialog()
extractor.py->>LinkedIn: page.evaluate() read dialog structure
LinkedIn-->>extractor.py: {step_count, questions, submit_button}
extractor.py->>extractor.py: _click_easy_apply_submit()
extractor.py->>LinkedIn: page.evaluate() click Submit
LinkedIn-->>extractor.py: clicked=true
Note over extractor.py,LinkedIn: JS loop exits immediately, window.Promise always truthy
extractor.py->>LinkedIn: page.evaluate() check dialog/badge no-op
LinkedIn-->>extractor.py: DOM snapshot, no real wait
extractor.py->>extractor.py: _dismiss_dialog()
extractor.py-->>applications.py: {status: submitted or submission_unconfirmed}
applications.py-->>Client: result dict
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+3191
to
+3201
| clicked = await self._page.evaluate( | ||
| """() => { | ||
| const btn = Array.from(document.querySelectorAll( | ||
| 'main button[aria-label*="Easy Apply"]' | ||
| )).find(b => !b.disabled | ||
| && (b.offsetWidth || b.offsetHeight || b.getClientRects().length)); | ||
| if (!btn) return false; | ||
| btn.click(); | ||
| return true; | ||
| }""" | ||
| ) |
Contributor
There was a problem hiding this comment.
easy_apply_label from _get_job_labels(), but the subsequent click still hardcodes "Easy Apply". Adding any non-English locale to _LOCALE_JOB_LABELS would detect the button correctly but fail to click it. Use the variable consistently.
Suggested change
| clicked = await self._page.evaluate( | |
| """() => { | |
| const btn = Array.from(document.querySelectorAll( | |
| 'main button[aria-label*="Easy Apply"]' | |
| )).find(b => !b.disabled | |
| && (b.offsetWidth || b.offsetHeight || b.getClientRects().length)); | |
| if (!btn) return false; | |
| btn.click(); | |
| return true; | |
| }""" | |
| ) | |
| clicked = await self._page.evaluate( | |
| f"""() => {{ | |
| const btn = Array.from(document.querySelectorAll( | |
| 'main button[aria-label*="{easy_apply_label}"]' | |
| )).find(b => !b.disabled | |
| && (b.offsetWidth || b.offsetHeight || b.getClientRects().length)); | |
| if (!btn) return false; | |
| btn.click(); | |
| return true; | |
| }}""" | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: linkedin_mcp_server/scraping/extractor.py
Line: 3191-3201
Comment:
**Easy Apply click selector ignores locale** — the detection step uses the locale-aware `easy_apply_label` from `_get_job_labels()`, but the subsequent click still hardcodes `"Easy Apply"`. Adding any non-English locale to `_LOCALE_JOB_LABELS` would detect the button correctly but fail to click it. Use the variable consistently.
```suggestion
clicked = await self._page.evaluate(
f"""() => {{
const btn = Array.from(document.querySelectorAll(
'main button[aria-label*="{easy_apply_label}"]'
)).find(b => !b.disabled
&& (b.offsetWidth || b.offsetHeight || b.getClientRects().length));
if (!btn) return false;
btn.click();
return true;
}}"""
)
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+307
to
+314
| def _get_job_labels() -> tuple[str, str]: | ||
| """Return (easy_apply, applied) labels for the current locale. | ||
|
|
||
| Falls back to English if locale is not in the table. | ||
| """ | ||
| # Could detect locale from page, but LinkedIn UI locale is usually English. | ||
| labels = _LOCALE_JOB_LABELS.get("en", ("Easy Apply", "Applied")) | ||
| return labels |
Contributor
There was a problem hiding this comment.
_get_job_labels() is effectively hardcoded to English. .get("en", ...) always succeeds because "en" is unconditionally present in _LOCALE_JOB_LABELS, making the fallback argument dead code and concealing the fact that locale detection hasn't been wired up. A future caller adding a new locale will get English labels silently. Add a locale parameter and a TODO comment.
Suggested change
| def _get_job_labels() -> tuple[str, str]: | |
| """Return (easy_apply, applied) labels for the current locale. | |
| Falls back to English if locale is not in the table. | |
| """ | |
| # Could detect locale from page, but LinkedIn UI locale is usually English. | |
| labels = _LOCALE_JOB_LABELS.get("en", ("Easy Apply", "Applied")) | |
| return labels | |
| def _get_job_labels(locale: str = "en") -> tuple[str, str]: | |
| """Return (easy_apply, applied) labels for the given locale. | |
| Falls back to English if locale is not in the table. | |
| """ | |
| # TODO: detect locale from the page (e.g. <html lang="..."> or | |
| # navigator.language) and pass it here instead of defaulting to "en". | |
| return _LOCALE_JOB_LABELS.get(locale, _LOCALE_JOB_LABELS["en"]) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: linkedin_mcp_server/scraping/extractor.py
Line: 307-314
Comment:
**`_get_job_labels()` is effectively hardcoded to English.** `.get("en", ...)` always succeeds because `"en"` is unconditionally present in `_LOCALE_JOB_LABELS`, making the fallback argument dead code and concealing the fact that locale detection hasn't been wired up. A future caller adding a new locale will get English labels silently. Add a `locale` parameter and a TODO comment.
```suggestion
def _get_job_labels(locale: str = "en") -> tuple[str, str]:
"""Return (easy_apply, applied) labels for the given locale.
Falls back to English if locale is not in the table.
"""
# TODO: detect locale from the page (e.g. <html lang="..."> or
# navigator.language) and pass it here instead of defaulting to "en".
return _LOCALE_JOB_LABELS.get(locale, _LOCALE_JOB_LABELS["en"])
```
How can I resolve this? If you propose a fix, please make it concise.- Add _get_next_button() to click "Continue to next step" - Add _has_next_step() to detect additional form steps - Add _auto_fill_fields() for form auto-fill with answers dict - Add _upload_resume_to_form() for resume upload handling - Add easy_apply_full() orchestrator for full multi-step flow
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
2 participants
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.
Summary
Address code review issues from PR #425:
destructiveHinttoTrueforsave_jobtoolb.type==='submit'fallback (risky for multi-step flows)Test plan
ruff checkpassesruff formatpasses