# PR: プロジェクト「課題・行動・成果」の統合 と 職務経歴書 UI 改善/ # PR: プロジェクトの在籍期間を複数対応（resume_project_periods 子テーブル化）#277
Merged
Conversation
# PR: プロジェクト「課題・行動・成果」の統合 と 職務経歴書 UI 改善
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR consolidates project narrative fields (challenge, action, result) into a single description field, models project periods as arrays, updates migrations/models/repos/schemas/renderers, refactors frontend types/payloads/forms/hooks/UI/CSS to the new shape, and replaces PDF panel maximize with a left-right resizable splitter. ChangesProject Description Consolidation & PDF Panel Layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 docstrings
🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/hooks/career/usePdfPanelLayout.ts (1)
63-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset global drag styles on unmount as well.
If unmount happens while dragging,
document.body.style.userSelect/cursorcan remain locked because cleanup removes listeners without restoring styles.🔧 Suggested fix
useEffect(() => { @@ return () => { window.removeEventListener("mousemove", onMove); window.removeEventListener("mouseup", onUp); + if (resizingRef.current) { + resizingRef.current = false; + document.body.style.userSelect = ""; + document.body.style.cursor = ""; + } }; }, [containerRef, minWidth, minFormWidth, reservedGap]);🤖 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 `@frontend/src/hooks/career/usePdfPanelLayout.ts` around lines 63 - 66, The cleanup returned from the drag effect currently removes event listeners but doesn't restore document-level styles, so if the component unmounts mid-drag userSelect/cursor can stay locked; update the cleanup in usePdfPanelLayout so it also resets document.body.style.userSelect and document.body.style.cursor back to their defaults (the same restoration logic used in onUp) when removing the "mousemove" and "mouseup" listeners for onMove and onUp to guarantee styles are cleared on unmount.frontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsx (1)
164-172:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace UI-only
min="0"with backend/payload validation foremployee_countandcapital.
backend/app/schemas/resume.pydefinesemployee_countandcapitalas free-formstrfields with onlymax_length(no numeric/min validators). Backend tests also allow empty values and strings like"300名"/"1億円", so negative or non-numeric inputs can still reach the repositories/models even withmin="0"on the HTML inputs.Add numeric parsing/normalization (enforce
>= 0, and likely integer foremployee_count) in the payload builder and/or the Pydantic schema, and adjust/update tests to match the intended accepted formats. Optionally addstep="1"foremployee_count.🤖 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 `@frontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsx` around lines 164 - 172, The UI uses min="0" on the employee_count input but validation/parsing must happen server-side and in payload construction: update the payload builder handling experiences (where onUpdateExperienceField / the form submission builds the payload) to parse/normalize employee_count (coerce numeric strings like "300名" to integer >=0, apply Math.floor/parseInt and default/omit on invalid) and normalize capital similarly (strip non-numeric characters, ensure >=0 or accept original formatted string per spec), add step="1" to the CareerExperienceEditor input for employee_count, and add corresponding validation to the Pydantic schema in backend/app/schemas/resume.py (either change employee_count to an int with ge=0 or add a custom validator that accepts the allowed formatted strings and enforces non-negative numeric values), then update tests to reflect the normalized/validated formats; reference CareerExperienceEditor, employee_count, capital, the payload builder function used on form submit, and backend/app/schemas/resume.py in your changes.
♻️ Duplicate comments (1)
frontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsx (1)
182-190:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame validation issue as employee_count above.
The
min="0"attribute on the capital field has the same insufficient validation issue noted in the employee_count comment above (lines 164-172). Frontend numeric validation should be added in the payload builder, and backend validation should be verified.🤖 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 `@frontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsx` around lines 182 - 190, The capital input's client-side min attribute is insufficient; update the payload builder that constructs the form submission (the function that reads values including exp.capital) to coerce and validate numeric fields (e.g., ensure capital is parsed to a non-negative number, reject or normalize invalid/empty values) before sending, mirror the same validation added for employee_count, and ensure onUpdateExperienceField still accepts string input but the payload conversion enforces the numeric check; also confirm the backend validates capital as a non-negative number.
🧹 Nitpick comments (2)
frontend/src/hooks/career/useProjectFormDirty.test.ts (1)
16-16: ⚡ Quick winAdd an explicit dirty check test for
description.This fixture change is good, but there’s still no direct assertion that
descriptiondiffs flipfields.descriptionandany.Proposed test addition
describe("useProjectFormDirty", () => { + it("description を変更すると fields.description と any が true", () => { + const original = buildProject({ description: "旧" }); + const local = buildProject({ description: "新" }); + const { result } = renderHook(() => useProjectFormDirty(local, original)); + expect(result.current.fields.description).toBe(true); + expect(result.current.any).toBe(true); + });🤖 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 `@frontend/src/hooks/career/useProjectFormDirty.test.ts` at line 16, Add an explicit unit test in frontend/src/hooks/career/useProjectFormDirty.test.ts that exercises the useProjectFormDirty hook's handling of the description field: render/use the hook (useProjectFormDirty) with a fixture that contains a non-empty description, then programmatically change the description to a different string and assert that state.fields.description === true and state.any === true; next, set description back to the original value (or to the same value as initial) and assert state.fields.description === false and state.any === false. Ensure the test references the hook by name (useProjectFormDirty) and checks the fields.description and fields.any flags directly.frontend/src/payloadBuilders.test.ts (1)
20-20: ⚡ Quick winAdd a focused test for
description-only project content.Given this schema shift, add one case where
nameis blank butdescriptionis non-empty to assert project inclusion/validation behavior explicitly.Proposed test addition
describe("buildCareerPayload (projects/clients/team)", () => { + it("name が空でも description に文字があれば project は中身ありとして扱う", () => { + const payload = buildCareerPayload( + baseState({ + experiences: [ + blankExperience({ + clients: [ + { + name: "顧客A", + has_client: true, + projects: [ + blankProject({ + name: " ", + description: "詳細あり", + start_date: "2024-01", + end_date: "2024-06", + is_current: false, + }), + ], + }, + ], + }), + ], + }), + ); + expect(payload.experiences[0].clients[0].projects).toHaveLength(1); + expect(payload.experiences[0].clients[0].projects[0].description).toBe("詳細あり"); + });🤖 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 `@frontend/src/payloadBuilders.test.ts` at line 20, Add a focused unit test in payloadBuilders.test.ts that covers a project object with an empty name and a non-empty description to assert inclusion/validation behavior; create a test case calling the same builder under test (e.g., buildPayload or buildProjectPayload) with a project like { name: "", description: "some text" } and assert the expected output/validation result (include expected project presence or error) to ensure description-only projects are handled explicitly.
🤖 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
`@backend/alembic_migrations/versions/0037_merge_project_caf_into_description.py`:
- Around line 34-36: The migration unconditionally drops legacy columns
"challenge", "action", and "result" using batch_op.drop_column which can
irreversibly lose data; add a fail-fast precondition before those drops in the
0037_merge_project_caf_into_description migration that queries the table for any
non-NULL values (e.g. use op.get_bind() / connection.execute to run SELECT
COUNT(*) FROM <table> WHERE challenge IS NOT NULL OR action IS NOT NULL OR
result IS NOT NULL) and if the count > 0 raise an exception or abort the
migration with a clear message so the destructive drops only run when the table
is confirmed empty of those columns' data.
---
Outside diff comments:
In `@frontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsx`:
- Around line 164-172: The UI uses min="0" on the employee_count input but
validation/parsing must happen server-side and in payload construction: update
the payload builder handling experiences (where onUpdateExperienceField / the
form submission builds the payload) to parse/normalize employee_count (coerce
numeric strings like "300名" to integer >=0, apply Math.floor/parseInt and
default/omit on invalid) and normalize capital similarly (strip non-numeric
characters, ensure >=0 or accept original formatted string per spec), add
step="1" to the CareerExperienceEditor input for employee_count, and add
corresponding validation to the Pydantic schema in backend/app/schemas/resume.py
(either change employee_count to an int with ge=0 or add a custom validator that
accepts the allowed formatted strings and enforces non-negative numeric values),
then update tests to reflect the normalized/validated formats; reference
CareerExperienceEditor, employee_count, capital, the payload builder function
used on form submit, and backend/app/schemas/resume.py in your changes.
In `@frontend/src/hooks/career/usePdfPanelLayout.ts`:
- Around line 63-66: The cleanup returned from the drag effect currently removes
event listeners but doesn't restore document-level styles, so if the component
unmounts mid-drag userSelect/cursor can stay locked; update the cleanup in
usePdfPanelLayout so it also resets document.body.style.userSelect and
document.body.style.cursor back to their defaults (the same restoration logic
used in onUp) when removing the "mousemove" and "mouseup" listeners for onMove
and onUp to guarantee styles are cleared on unmount.
---
Duplicate comments:
In `@frontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsx`:
- Around line 182-190: The capital input's client-side min attribute is
insufficient; update the payload builder that constructs the form submission
(the function that reads values including exp.capital) to coerce and validate
numeric fields (e.g., ensure capital is parsed to a non-negative number, reject
or normalize invalid/empty values) before sending, mirror the same validation
added for employee_count, and ensure onUpdateExperienceField still accepts
string input but the payload conversion enforces the numeric check; also confirm
the backend validates capital as a non-negative number.
---
Nitpick comments:
In `@frontend/src/hooks/career/useProjectFormDirty.test.ts`:
- Line 16: Add an explicit unit test in
frontend/src/hooks/career/useProjectFormDirty.test.ts that exercises the
useProjectFormDirty hook's handling of the description field: render/use the
hook (useProjectFormDirty) with a fixture that contains a non-empty description,
then programmatically change the description to a different string and assert
that state.fields.description === true and state.any === true; next, set
description back to the original value (or to the same value as initial) and
assert state.fields.description === false and state.any === false. Ensure the
test references the hook by name (useProjectFormDirty) and checks the
fields.description and fields.any flags directly.
In `@frontend/src/payloadBuilders.test.ts`:
- Line 20: Add a focused unit test in payloadBuilders.test.ts that covers a
project object with an empty name and a non-empty description to assert
inclusion/validation behavior; create a test case calling the same builder under
test (e.g., buildPayload or buildProjectPayload) with a project like { name: "",
description: "some text" } and assert the expected output/validation result
(include expected project presence or error) to ensure description-only projects
are handled explicitly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 13082f7d-048e-4ffe-827e-9e22f82b0a6b
📒 Files selected for processing (33)
backend/alembic_migrations/versions/0037_merge_project_caf_into_description.pybackend/app/models/resume.pybackend/app/repositories/resume.pybackend/app/schemas/resume.pybackend/app/services/markdown/generators/resume_generator.pybackend/app/services/pdf/generators/resume_generator.pybackend/tests/test_delete_documents.pybackend/tests/test_endpoints.pybackend/tests/test_schemas.pyfrontend/e2e/career-dirty-indicator.spec.tsfrontend/src/App.module.cssfrontend/src/components/forms/CareerFormEditors/CareerExperienceEditor.tsxfrontend/src/components/forms/CareerResumeForm.module.cssfrontend/src/components/forms/CareerResumeForm.tsxfrontend/src/components/forms/ProjectModal.module.cssfrontend/src/components/forms/ProjectModal.test.tsxfrontend/src/components/forms/ProjectModal.tsxfrontend/src/components/forms/ResumePdfTracePanel.module.cssfrontend/src/components/forms/ResumePdfTracePanel.tsxfrontend/src/constants.tsfrontend/src/constants/messages.tsfrontend/src/formTypes.tsfrontend/src/hooks/career/usePdfPanelLayout.test.tsfrontend/src/hooks/career/usePdfPanelLayout.tsfrontend/src/hooks/career/useProjectFormDirty.test.tsfrontend/src/hooks/career/useProjectFormDirty.tsfrontend/src/hooks/career/useProjectModalForm.test.tsfrontend/src/hooks/career/useProjectModalState.test.tsfrontend/src/payloadBuilders.test.tsfrontend/src/payloadBuilders.tsfrontend/src/styles.cssfrontend/src/types.tsfrontend/tests/payloadBuilders.test.cjs
💤 Files with no reviewable changes (1)
- frontend/src/constants/messages.ts
project date list add
yusuke0610
changed the title
Owner
Author
Summary職務経歴書（Resume）のプロジェクトが持つ在籍期間を、1 案件 = 1 期間から 1 案件 = 複数期間へ拡張する PR。 これまで あわせて、本 PR とは独立した文書追加として ADR-0006「TanStack Query 導入検討」(Proposed) を含む。 1. データモデル: 期間の子テーブル化 (Backend)マイグレーション
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/services/markdown/generators/resume_generator.py (1)
94-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender
descriptionas a block instead of an inline field line.
descriptionnow comes from aMarkdownTextarea, so it can contain multi-paragraph Markdown, lists, or tables. Wrapping it withfield_line("詳細", description)prepends**詳細:**to the first line and breaks block-level Markdown rendering for the new field.Suggested change
description = _a(proj, "description") if description: - lines.append(field_line("詳細", description)) + lines.append("**詳細:**") + lines.append("") + lines.append(description)🤖 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 `@backend/app/services/markdown/generators/resume_generator.py` around lines 94 - 96, The current code uses field_line("詳細", description) which forces the description into an inline field and breaks block-level Markdown; instead, stop calling field_line for the description variable and render it as a block: append a block label/header (e.g. a bold "詳細" or a heading) to lines, ensure a blank line after the header, then append the raw description content (preserving its multi-line Markdown) and a trailing blank line so lists/tables render correctly; update the handling where proj and description are used and remove the field_line("詳細", ...) usage.backend/app/schemas/resume.py (1)
69-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the
YYYY-MMshape before this reachesparse_year_month().
validate_dates()only checks blank/current/range. Payloads like"2024/1"or"foo"still pass here, thenbackend/app/repositories/resume.pycallsparse_year_month(period["start_date"])and turns that bad input into a 500. Add a format validator (regex or parse-based) onProjectPeriodso malformed API payloads fail as 422 at the schema boundary.🤖 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 `@backend/app/schemas/resume.py` around lines 69 - 90, ProjectPeriod.validate_dates currently only checks presence and ordering but allows malformed strings; update validate_dates to assert that start_date (and end_date when is_current is False) match the YYYY-MM shape before any parsing (e.g. use a regex like ^\d{4}-(0[1-9]|1[0-2])$ or datetime.strptime check) and raise a ValueError via get_error("validation.invalid_year_month") (or an appropriate existing key) if the format is wrong so bad payloads fail with 422 at the schema boundary rather than bubbling into parse_year_month.
🧹 Nitpick comments (2)
frontend/src/hooks/career/useProjectModalForm.ts (1)
59-73: ⚡ Quick winTighten
updatePeriodFieldtyping to prevent future truthy-coercion misuseCurrent
"is_current"call sites already pass booleans (e.g.,e.target.value === "current"and test usestrue), so the"false"→truetrap won’t occur today. Still, thevalue: string | booleansignature makes it easy to introduce this bug later—constrain the type based on thekey.💡 Suggested fix
- const updatePeriodField = ( - periodIndex: number, - key: CareerProjectPeriodFieldKey, - value: string | boolean, - ) => { + const updatePeriodField = <K extends CareerProjectPeriodFieldKey>( + periodIndex: number, + key: K, + value: CareerProjectPeriodForm[K], + ) => { setLocal((prev) => ({ ...prev, periods: prev.periods.map((p, i): CareerProjectPeriodForm => { if (i !== periodIndex) return p; if (key === "is_current") { - const isCurrent = Boolean(value); + const isCurrent = value as boolean; return { ...p, is_current: isCurrent, end_date: isCurrent ? "" : p.end_date }; } - return { ...p, [key]: value }; + return { ...p, [key]: value } as CareerProjectPeriodForm; }), })); };🤖 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 `@frontend/src/hooks/career/useProjectModalForm.ts` around lines 59 - 73, The updatePeriodField currently accepts value: string | boolean which allows accidental truthy coercion for the "is_current" key; change its signature to use two overloads (or a conditional/discriminated union) so that when key is "is_current" value must be boolean and for other CareerProjectPeriodFieldKey values value must be string, then keep the implementation body (setLocal and the periods.map logic) the same but rely on the narrowed types so the branch that handles key === "is_current" uses a boolean without coercion; reference: updatePeriodField, CareerProjectPeriodFieldKey, and the setLocal/periods.map block.frontend/tests/payloadBuilders.test.cjs (1)
27-29: ⚡ Quick winAssert the new
periodsmapping in this regression test.This fixture was updated to the new shape, but the test never checks
projects[0].periods. A regression inbuildPeriodwould still pass here.Suggested test addition
- { start_date: "2020-04", end_date: "2021-03", is_current: false } + { start_date: " 2020-04 ", end_date: " 2021-03 ", is_current: false } ], @@ assert.equal(payload.experiences[0].clients[0].projects[0].name, "プロジェクトA"); + assert.deepEqual(payload.experiences[0].clients[0].projects[0].periods, [ + { start_date: "2020-04", end_date: "2021-03", is_current: false } + ]); assert.equal(payload.experiences[0].clients[0].projects[0].role, "メンバー");Also applies to: 77-89
🤖 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 `@frontend/tests/payloadBuilders.test.cjs` around lines 27 - 29, Add assertions in the test file frontend/tests/payloadBuilders.test.cjs to verify the new periods mapping produced by buildPeriod: after building payloads, assert projects[0].periods strictly equals the expected array shape (e.g. [{ start_date: "2020-04", end_date: "2021-03", is_current: false }]); do the same for the other fixture case referenced around the 77-89 block so both tests validate projects[0].periods rather than only relying on the fixture shape. Use the existing test helpers/assertion style in the file to compare the periods array.
🤖 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 `@backend/app/schemas/resume.py`:
- Around line 93-125: The Project model must guarantee at least one period:
change the periods Field declaration in class Project so its default is a
single-element list (e.g., default_factory=lambda: [ProjectPeriod()]) and/or add
Field(min_items=1) to enforce non-empty lists; keep the _migrate_legacy_fields
logic (which still populates periods when start_date is present) but ensure
creating Project() or validation without legacy dates yields one empty
ProjectPeriod instead of an empty list so ResumeRepository._build_project_row()
always sees at least one period.
In `@frontend/src/payloadBuilders.ts`:
- Around line 177-193: The current validation loop over proj.periods only checks
existing entries and misses the case where proj.periods is empty; ensure
projects with zero periods are rejected by adding a check before the loop that
throws VALIDATION_MESSAGES.PROJECT_START_DATE_REQUIRED or a new message (e.g.,
PROJECT_PERIODS_REQUIRED) when proj.periods is missing or has length === 0, then
continue to validate each period as done in the existing block (references:
proj.periods, VALIDATION_MESSAGES.* and the period validation logic).
---
Outside diff comments:
In `@backend/app/schemas/resume.py`:
- Around line 69-90: ProjectPeriod.validate_dates currently only checks presence
and ordering but allows malformed strings; update validate_dates to assert that
start_date (and end_date when is_current is False) match the YYYY-MM shape
before any parsing (e.g. use a regex like ^\d{4}-(0[1-9]|1[0-2])$ or
datetime.strptime check) and raise a ValueError via
get_error("validation.invalid_year_month") (or an appropriate existing key) if
the format is wrong so bad payloads fail with 422 at the schema boundary rather
than bubbling into parse_year_month.
In `@backend/app/services/markdown/generators/resume_generator.py`:
- Around line 94-96: The current code uses field_line("詳細", description) which
forces the description into an inline field and breaks block-level Markdown;
instead, stop calling field_line for the description variable and render it as a
block: append a block label/header (e.g. a bold "詳細" or a heading) to lines,
ensure a blank line after the header, then append the raw description content
(preserving its multi-line Markdown) and a trailing blank line so lists/tables
render correctly; update the handling where proj and description are used and
remove the field_line("詳細", ...) usage.
---
Nitpick comments:
In `@frontend/src/hooks/career/useProjectModalForm.ts`:
- Around line 59-73: The updatePeriodField currently accepts value: string |
boolean which allows accidental truthy coercion for the "is_current" key; change
its signature to use two overloads (or a conditional/discriminated union) so
that when key is "is_current" value must be boolean and for other
CareerProjectPeriodFieldKey values value must be string, then keep the
implementation body (setLocal and the periods.map logic) the same but rely on
the narrowed types so the branch that handles key === "is_current" uses a
boolean without coercion; reference: updatePeriodField,
CareerProjectPeriodFieldKey, and the setLocal/periods.map block.
In `@frontend/tests/payloadBuilders.test.cjs`:
- Around line 27-29: Add assertions in the test file
frontend/tests/payloadBuilders.test.cjs to verify the new periods mapping
produced by buildPeriod: after building payloads, assert projects[0].periods
strictly equals the expected array shape (e.g. [{ start_date: "2020-04",
end_date: "2021-03", is_current: false }]); do the same for the other fixture
case referenced around the 77-89 block so both tests validate
projects[0].periods rather than only relying on the fixture shape. Use the
existing test helpers/assertion style in the file to compare the periods array.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 73c0ed99-3634-44d8-b7b2-8b37e5cb22e1
📒 Files selected for processing (25)
CONTRIBUTING.mdbackend/alembic_migrations/versions/0038_add_project_periods_table.pybackend/app/models/__init__.pybackend/app/models/resume.pybackend/app/repositories/resume.pybackend/app/schemas/resume.pybackend/app/services/markdown/generators/resume_generator.pybackend/app/services/pdf/generators/resume_generator.pybackend/tests/test_schemas.pydocs/adr/0006-tanstack-query.mdfrontend/src/components/forms/ProjectModal.test.tsxfrontend/src/components/forms/ProjectModal.tsxfrontend/src/components/forms/sections/CareerExperienceSection.tsxfrontend/src/constants.tsfrontend/src/formMappers.tsfrontend/src/formTypes.tsfrontend/src/hooks/career/useProjectFormDirty.test.tsfrontend/src/hooks/career/useProjectFormDirty.tsfrontend/src/hooks/career/useProjectModalForm.test.tsfrontend/src/hooks/career/useProjectModalForm.tsfrontend/src/hooks/career/useProjectModalState.test.tsfrontend/src/payloadBuilders.test.tsfrontend/src/payloadBuilders.tsfrontend/src/types.tsfrontend/tests/payloadBuilders.test.cjs
✅ Files skipped from review due to trivial changes (3)
- CONTRIBUTING.md
- backend/app/models/init.py
- docs/adr/0006-tanstack-query.md
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/forms/ProjectModal.test.tsx
- frontend/src/hooks/career/useProjectFormDirty.ts
- frontend/src/hooks/career/useProjectModalState.test.ts
Comment on lines
+93
to
+125
| class Project(BaseModel): | ||
| name: str = Field(max_length=200, default="") | ||
| periods: list[ProjectPeriod] = Field(default_factory=list) | ||
| role: str = Field(max_length=200, default="") | ||
| # 課題・行動・成果を統合した自由記述欄（見出し「詳細」） | ||
| description: str = Field(max_length=4500, default="") | ||
| team: ProjectTeam = Field(default_factory=ProjectTeam) | ||
| technology_stacks: list[TechnologyStackItem] = Field(default_factory=list) | ||
| phases: list[str] = Field(default_factory=list) | ||
|
|
||
| model_config = ConfigDict(from_attributes=True) | ||
|
|
||
| @model_validator(mode="before") | ||
| @classmethod | ||
| def _migrate_legacy_fields(cls, data: dict) -> dict: | ||
| """旧形式の後方互換処理。 | ||
| - scale → team | ||
| - start_date/end_date/is_current（フラット）→ periods[0] | ||
| """ | ||
| if not isinstance(data, dict): | ||
| return data | ||
| if "scale" in data and "team" not in data: | ||
| scale = data.pop("scale") | ||
| data["team"] = {"total": str(scale) if scale else "", "members": []} | ||
| if "start_date" in data and "periods" not in data: | ||
| data["periods"] = [ | ||
| { | ||
| "start_date": data.pop("start_date", ""), | ||
| "end_date": data.pop("end_date", ""), | ||
| "is_current": data.pop("is_current", False), | ||
| } | ||
| ] | ||
| return data |
There was a problem hiding this comment.
Make periods non-empty again.
Project used to guarantee at least one date range via the flat start_date field. With periods: list[ProjectPeriod] = Field(default_factory=list), Project(periods=[]) now validates, and ResumeRepository._build_project_row() will persist a project with no period_rows, so period-based sorting/rendering silently loses its duration. Please require at least one period here instead of defaulting to an empty list.
🤖 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 `@backend/app/schemas/resume.py` around lines 93 - 125, The Project model must
guarantee at least one period: change the periods Field declaration in class
Project so its default is a single-element list (e.g., default_factory=lambda:
[ProjectPeriod()]) and/or add Field(min_items=1) to enforce non-empty lists;
keep the _migrate_legacy_fields logic (which still populates periods when
start_date is present) but ensure creating Project() or validation without
legacy dates yields one empty ProjectPeriod instead of an empty list so
ResumeRepository._build_project_row() always sees at least one period.
|
Actionable comments posted: 0 |
This was referenced Jun 1, 2026
This was referenced Jun 30, 2026
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.
Summary
職務経歴書（Resume）プロジェクト欄の入力モデルを単純化し、あわせて職務経歴書ページ／プロジェクトモーダルの UI を整理した PR。大きく 2 つの変更からなる。
description) に統合1. 課題・行動・成果 → 「詳細」へ統合 (
73eed31)プロジェクトの「課題」「行動」「成果」は分割する必要がないと判断し、単一の自由記述欄「詳細」(
description) に統合した。Backend
0037_merge_project_caf_into_descriptionを新設。resume_projectsのchallenge/action/resultを DROP し、description(Text, NOT NULL, default "") を追加。DROP COLUMNを直接サポートしないためbatch_alter_table（テーブル再作成）で入れ替え。down_revision: 0036_...）。app/models/resume.py—ResumeProjectのカラム定義をdescriptionに置換。app/schemas/resume.py—Projectスキーマをdescription: str (max_length=4500)に変更（旧 3 欄の 1500×3 を集約）。app/repositories/resume.py— payload マッピングをdescriptionに統一。app/services/markdown/generators/resume_generator.py/app/services/pdf/generators/resume_generator.py— 出力見出しを課題 / 行動 / 成果の 3 ブロックから詳細単一ブロックに変更。Frontend
descriptionに統一:types.ts(CareerProject)、formTypes.ts(CareerProjectFieldKey)、payloadBuilders.ts(CareerProjectForm+buildProject+ 非空フィルタhasAnyText([p.name, p.description]))、constants.ts(blankCareerProject)。ProjectModal.tsx— 3 つのMarkdownTextarea（課題/行動/成果, rows=2）を 1 つの「詳細」(rows=6) に置換。hooks/career/useProjectFormDirty.ts— dirty 判定フィールドをdescriptionに統一。テスト更新
test_delete_documents.py/test_endpoints.py/test_schemas.pyProjectModal.test.tsx/useProjectFormDirty.test.ts/useProjectModalForm.test.ts/useProjectModalState.test.ts/payloadBuilders.test.ts/payloadBuilders.test.cjs/e2e/career-dirty-indicator.spec.tsdescriptionへ更新（payloadBuilders.test.cjsはdescriptionの trim を検証する assert を追加）。2. 職務経歴書 UI 改善 (
4967105)レイアウト定数の CSS 変数化
styles.cssに--sidebar-width: 220px/--topbar-height: 48pxを追加し、App.module.cssのサイドバー幅・メインコンテンツ余白・モバイル上部バー高を変数参照に統一。--warning-text/--warning-bg/--warning-borderをライト／ダーク両方に定義。ダークモード対応（ハードコード色の排除）
ResumePdfTracePanel.module.cssの固定色（#f1f3f5等）を CSS 変数に置換。ズームボタンはグローバルのbutton { color:#fff }を打ち消すためcolorを明示。プロジェクトモーダルの全面化 + スプリッター
ProjectModal.module.css/ProjectModal.tsx— モーダルを固定幅（max-width:800px/modalWide:1150px）からコンテンツ領域いっぱい（left: var(--sidebar-width)起点で全幅・全高）に変更。サイドバーは覆わず操作可能なまま残す。usePdfPanelLayoutでドラッグ左右リサイズ（initialWidth:440等）。狭幅時（max-width:700px）は縦積み・スプリッター非表示に切替。モバイル（max-width:768px）は上部バー下から全幅表示。PDF パネルの「最大化」機能を削除
usePdfPanelLayout.tsからmaximizedstate とtoggleMaximizeを削除し、リサイズのみに簡素化。ResumePdfTracePanel.tsxから最大化ボタン／ダブルクリック・関連 props を削除。CareerResumeForm.tsxの利用箇所・CareerResumeForm.module.cssの.maximizedCSS・messages.tsのMAXIMIZE/RESTORE/MAXIMIZE_HINTを削除。その他
min="0"を付与（CareerExperienceEditorの従業員数・資本金、ProjectModalの体制人数・メンバー人数）。CareerResumeForm.module.cssの splitter hover 色をvar(--accent)に統一。テスト更新
usePdfPanelLayout.test.ts— 最大化トグルの検証を削除し「初期幅を返す」に縮小。Test plan
make migrate（0037の up/down が libSQL で通る）make ci（lint + test + build-frontend）make test-backend— Resume の round-trip / 削除 / スキーマテストmake test-frontend— ProjectModal / dirty / payloadBuildersnpm run test:e2e）— モーダル全面化・スプリッター・PDF パネル補足
0037のdescriptionはserver_default=""付きで追加するため、既存行があっても NOT NULL 制約に抵触しない。Summary by CodeRabbit
New Features
New Inputs / UI
Improvements